Ship Reflex integration as xy[reflex] - #408
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR bundles the Reflex integration into ChangesBundled Reflex integration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Merging this PR will not alter performance
Comparing 
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
spec/design/reflex-shaped-api.md (1)
556-581: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAlign this section with the shipped integration.
These lines describe bundled
reflex_xyandxy[reflex], but the same document still presents the adapter as future work and recommends adding a separate adapter package. Update the surrounding section so it describes the current bundled integration. Mark smaller-dependency or separate-package work as future work only.As per coding guidelines, the
spec/directory must remain current with relevant code, configuration, build, and release changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/design/reflex-shaped-api.md` around lines 556 - 581, Update the surrounding design section to present the bundled reflex_xy integration and xy[reflex] extra as the current shipped implementation, removing stale future-work language and separate-adapter-package recommendations. Retain smaller dependency surfaces or extracting a separate package only as explicitly identified future work, and ensure the documented behavior matches the relevant code, configuration, build, and release metadata.Source: Coding guidelines
🧹 Nitpick comments (2)
python/reflex_xy/component.py (1)
205-220: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider removing the implicit dependency on
chartinitializing_component_cls.
_facet_gridcalls_component_cls.create(...)but never initializes_component_cls. Today the only caller ischart, which builds the class first, so the code is correct. Any new caller of_facet_gridwould raiseAttributeError: 'NoneType' object has no attribute 'create'. Extract the lazy initialization into a small helper and call it in both places.♻️ Proposed helper
+def _component() -> Any: + global _component_cls + if _component_cls is None: + _component_cls = _build_component_cls() + return _component_clsThen use
_component().create(...)in_facet_gridand inchart.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/reflex_xy/component.py` around lines 205 - 220, Extract lazy _component_cls initialization into a shared _component() helper, then replace direct _component_cls.create calls with _component().create in both _facet_grid and chart. Preserve the existing class construction behavior while ensuring _facet_grid works independently of chart initialization.python/reflex_xy/assets/XYChart.jsx (1)
361-380: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider aborting the static payload fetch on unmount.
The
cancelledflag suppresses the result, but the request keeps running after unmount. AnAbortControllerreleases the connection immediately, which matters for pages that mount and unmount many static charts.♻️ Proposed change
let cancelled = false; + const controller = new AbortController(); const handleViewChange = (event) => cbRef.current.onViewChange?.(event.detail); el.addEventListener("xy:view_change", handleViewChange); - fetch(src) + fetch(src, { signal: controller.signal }) .then((resp) => {.catch((err) => { - if (!cancelled) console.warn(`xy: static payload failed for ${src}`, err); + if (!cancelled) console.warn(`xy: static payload failed for ${src}`, err); }); return () => { cancelled = true; + controller.abort();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/reflex_xy/assets/XYChart.jsx` around lines 361 - 380, Update the static payload fetch flow in the visible effect around fetch(src) to create an AbortController, pass its signal to fetch, and abort it during unmount cleanup. Preserve the existing cancelled checks and ensure abort errors remain safely handled by the current catch path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 127-129: Update the smoke-test Python commands in the release
workflow to import importlib.metadata and assert that reflex_xy.__version__
equals importlib.metadata.version("xy"), while preserving the existing native
backend assertion and output behavior in both Unix and Windows fallbacks.
In `@docs/integrations/reflex.md`:
- Around line 20-21: Update the installation text in the Reflex integration
documentation to say that the extra installs the supported Reflex dependency
floor, reflecting the metadata constraint reflex>=0.9.6 rather than implying a
bounded compatible version. Revise the warning around the package’s PyPI
availability and adapter tag requirements to describe the bundled distribution
accurately, removing claims that it has no PyPI release or requires a matching
adapter tag.
In `@python/reflex_xy/events.py`:
- Around line 38-206: Move all numeric semantic-event payloads in
python/reflex_xy/events.py:38-206—including point coordinates, selection
bounds/polygons, row IDs, row projections, and counts—into a defined binary
event ABI, leaving only non-numeric metadata and attachment references in JSON;
update PointData, ScreenPoint, DataBounds, SelectionPayload, and the event
TypedDicts consistently. In python/reflex_xy/selections.py:17-19 and 37-42,
decode the binary selection geometry before calling select_range or
select_polygon, and update resolve_selection() to consume the binary IDs, rows,
and counts while preserving truncation and clear-selection behavior.
In `@python/reflex_xy/namespace.py`:
- Around line 153-167: Add a FigureRegistry operation that atomically snapshots
each entry’s figure and version under the registry mutex, coordinated with
publish. Update on_sub and the other affected handlers to use that snapshot for
payload building and message processing instead of reading entry.figure and
entry.version independently. Before emitting any interaction reply, compare the
current registry version with the captured version and discard the reply when
they differ.
- Around line 132-141: Update on_connect to validate the query-string token
through Reflex’s server-side session manager before calling save_session. Reject
the connection when the token is missing or validation fails, and only store the
validated token for authenticated connections; use the existing
session-validation symbol and connection rejection mechanism already available
in the namespace.
In `@python/reflex_xy/payload_asset.py`:
- Around line 76-82: Update the temporary path construction in the asset-writing
block around dest and tmp so each writer gets a unique filename, using the
process ID plus a random or otherwise collision-resistant suffix. Keep the
existing write_bytes and replace flow, ensuring concurrent writers never share
the same temporary file while publishing the identical final dest.
In `@python/reflex_xy/registry.py`:
- Around line 111-127: Synchronize figure replacement in publish with
entry.lock, using the same generation-safe path as payload construction and
append mutation. Keep figure replacement, version increment, payload or delta
construction, and version capture coordinated so each emitted message references
the exact figure or delta used to build it, preventing stale figures or
incompatible deltas.
In `@python/reflex_xy/state_bridge.py`:
- Around line 63-65: Update the chart conversion flow around _figure_of so its
result is validated as a Figure before publication. If the converted value is
invalid, return None or raise the established controlled builder error; preserve
the existing None handling and only publish valid Figure instances.
In `@scripts/verify_sdist.py`:
- Around line 257-274: Require exactly one valid Reflex requirement for the
reflex extra in scripts/verify_sdist.py:257-274 and
scripts/verify_wheel.py:202-224, rejecting duplicate or conflicting constraints
while preserving the existing minimum-version validation. Add conflicting
second-Reflex-requirement cases in tests/test_verify_sdist.py:231-248 and
tests/test_verify_wheel.py:279-296 to verify both artifact verifiers reject the
metadata.
In `@spec/api/export.md`:
- Line 51: Update the user-facing wording in the export documentation by
replacing “kernel-lessly” with “without a kernel,” while preserving the
surrounding statement and meaning.
In `@spec/process/production-readiness.md`:
- Around line 386-390: Update the wheel verification checklist to apply
scripts/verify_wheel.py --expect-native only to native wheels, and add a
separate fallback-wheel verification using --expect-pure. Keep the release
checks aligned with the native and fallback gates described earlier in the
document.
In `@tests/reflex_adapter/conftest.py`:
- Line 5: Replace the malformed comment text “importorskips” in the Reflex
dependency-rule note with clear wording that directly states these tests are
skipped when the Reflex dependency cannot be imported.
---
Outside diff comments:
In `@spec/design/reflex-shaped-api.md`:
- Around line 556-581: Update the surrounding design section to present the
bundled reflex_xy integration and xy[reflex] extra as the current shipped
implementation, removing stale future-work language and separate-adapter-package
recommendations. Retain smaller dependency surfaces or extracting a separate
package only as explicitly identified future work, and ensure the documented
behavior matches the relevant code, configuration, build, and release metadata.
---
Nitpick comments:
In `@python/reflex_xy/assets/XYChart.jsx`:
- Around line 361-380: Update the static payload fetch flow in the visible
effect around fetch(src) to create an AbortController, pass its signal to fetch,
and abort it during unmount cleanup. Preserve the existing cancelled checks and
ensure abort errors remain safely handled by the current catch path.
In `@python/reflex_xy/component.py`:
- Around line 205-220: Extract lazy _component_cls initialization into a shared
_component() helper, then replace direct _component_cls.create calls with
_component().create in both _facet_grid and chart. Preserve the existing class
construction behavior while ensuring _facet_grid works independently of chart
initialization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 522321d0-e1ab-4f9c-9f27-abaa3e094c27
⛔ Files ignored due to path filters (4)
docs/app/reflex.lock/bun.lockis excluded by!**/*.lockdocs/app/uv.lockis excluded by!**/*.lockpython/reflex-xy/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (84)
.github/workflows/ci.yml.github/workflows/release-reflex-xy.yml.github/workflows/release.ymlCHANGELOG.mdCLAUDE.mdMakefileREADME.mddocs/advanced/runtime-and-deployment.mddocs/api-reference/events-and-callbacks.mddocs/app/pyproject.tomldocs/app/reflex.lock/package.jsondocs/app/rxconfig.pydocs/app/tests/test_docs_site.pydocs/app/xy_docs/api_reference.pydocs/app/xy_docs/demos/benchmark_charts.pydocs/app/xy_docs/demos/xy_sdf_plots.pydocs/app/xy_docs/playground.pydocs/guides/deployment-recipes.mddocs/integrations/index.mddocs/integrations/reflex.mddocs/overview/installation.mdexamples/reflex/README.mdexamples/reflex/assets/.gitkeepexamples/reflex/pyproject.tomlexamples/reflex/rxconfig.pyexamples/reflex/xy_reflex_demo/__init__.pyexamples/reflex/xy_reflex_demo/xy_reflex_demo.pypyproject.tomlpython/reflex-xy/CHANGELOG.mdpython/reflex-xy/README.mdpython/reflex-xy/pyproject.tomlpython/reflex_xy/__init__.pypython/reflex_xy/app.pypython/reflex_xy/assets/XYChart.jsxpython/reflex_xy/assets/__init__.pypython/reflex_xy/component.pypython/reflex_xy/events.pypython/reflex_xy/namespace.pypython/reflex_xy/payload_asset.pypython/reflex_xy/registry.pypython/reflex_xy/selections.pypython/reflex_xy/state_bridge.pypython/reflex_xy/tokens.pypython/reflex_xy/vars.pyscripts/check_release_version.pyscripts/reflex_ws_smoke.pyscripts/verify_ci_workflow.pyscripts/verify_reflex_xy_dist.pyscripts/verify_sdist.pyscripts/verify_wheel.pyspec/README.mdspec/api/chart-roadmap.mdspec/api/export.mdspec/api/interaction.mdspec/design/chart-grammar.mdspec/design/reflex-integration.mdspec/design/reflex-shaped-api.mdspec/design/renderer-architecture.mdspec/design/view-state.mdspec/design/wire-protocol.mdspec/process/contributing.mdspec/process/production-readiness.mdtests/reflex_adapter/__init__.pytests/reflex_adapter/conftest.pytests/reflex_adapter/test_assets.pytests/reflex_adapter/test_async_figure_var.pytests/reflex_adapter/test_component.pytests/reflex_adapter/test_figure_var.pytests/reflex_adapter/test_payload_asset.pytests/reflex_adapter/test_registry.pytests/reflex_adapter/test_selections.pytests/reflex_adapter/test_socket_data_plane.pytests/reflex_adapter/test_state_bridge.pytests/reflex_adapter/test_tokens.pytests/reflex_adapter/test_view_state_push.pytests/test_check_release_version.pytests/test_dependencies.pytests/test_payload_update_rehome.pytests/test_tailwind_root_customization.pytests/test_verify_ci_workflow.pytests/test_verify_local.pytests/test_verify_reflex_xy_dist.pytests/test_verify_sdist.pytests/test_verify_wheel.py
💤 Files with no reviewable changes (8)
- docs/app/reflex.lock/package.json
- .github/workflows/release-reflex-xy.yml
- python/reflex-xy/README.md
- python/reflex-xy/CHANGELOG.md
- python/reflex-xy/pyproject.toml
- scripts/verify_reflex_xy_dist.py
- tests/test_verify_ci_workflow.py
- tests/test_verify_reflex_xy_dist.py
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
spec/design/reflex-shaped-api.md (1)
556-581: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAlign this section with the shipped integration.
These lines describe bundled
reflex_xyandxy[reflex], but the same document still presents the adapter as future work and recommends adding a separate adapter package. Update the surrounding section so it describes the current bundled integration. Mark smaller-dependency or separate-package work as future work only.As per coding guidelines, the
spec/directory must remain current with relevant code, configuration, build, and release changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/design/reflex-shaped-api.md` around lines 556 - 581, Update the surrounding design section to present the bundled reflex_xy integration and xy[reflex] extra as the current shipped implementation, removing stale future-work language and separate-adapter-package recommendations. Retain smaller dependency surfaces or extracting a separate package only as explicitly identified future work, and ensure the documented behavior matches the relevant code, configuration, build, and release metadata.Source: Coding guidelines
🧹 Nitpick comments (2)
python/reflex_xy/component.py (1)
205-220: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider removing the implicit dependency on
chartinitializing_component_cls.
_facet_gridcalls_component_cls.create(...)but never initializes_component_cls. Today the only caller ischart, which builds the class first, so the code is correct. Any new caller of_facet_gridwould raiseAttributeError: 'NoneType' object has no attribute 'create'. Extract the lazy initialization into a small helper and call it in both places.♻️ Proposed helper
+def _component() -> Any: + global _component_cls + if _component_cls is None: + _component_cls = _build_component_cls() + return _component_clsThen use
_component().create(...)in_facet_gridand inchart.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/reflex_xy/component.py` around lines 205 - 220, Extract lazy _component_cls initialization into a shared _component() helper, then replace direct _component_cls.create calls with _component().create in both _facet_grid and chart. Preserve the existing class construction behavior while ensuring _facet_grid works independently of chart initialization.python/reflex_xy/assets/XYChart.jsx (1)
361-380: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider aborting the static payload fetch on unmount.
The
cancelledflag suppresses the result, but the request keeps running after unmount. AnAbortControllerreleases the connection immediately, which matters for pages that mount and unmount many static charts.♻️ Proposed change
let cancelled = false; + const controller = new AbortController(); const handleViewChange = (event) => cbRef.current.onViewChange?.(event.detail); el.addEventListener("xy:view_change", handleViewChange); - fetch(src) + fetch(src, { signal: controller.signal }) .then((resp) => {.catch((err) => { - if (!cancelled) console.warn(`xy: static payload failed for ${src}`, err); + if (!cancelled) console.warn(`xy: static payload failed for ${src}`, err); }); return () => { cancelled = true; + controller.abort();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/reflex_xy/assets/XYChart.jsx` around lines 361 - 380, Update the static payload fetch flow in the visible effect around fetch(src) to create an AbortController, pass its signal to fetch, and abort it during unmount cleanup. Preserve the existing cancelled checks and ensure abort errors remain safely handled by the current catch path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 127-129: Update the smoke-test Python commands in the release
workflow to import importlib.metadata and assert that reflex_xy.__version__
equals importlib.metadata.version("xy"), while preserving the existing native
backend assertion and output behavior in both Unix and Windows fallbacks.
In `@docs/integrations/reflex.md`:
- Around line 20-21: Update the installation text in the Reflex integration
documentation to say that the extra installs the supported Reflex dependency
floor, reflecting the metadata constraint reflex>=0.9.6 rather than implying a
bounded compatible version. Revise the warning around the package’s PyPI
availability and adapter tag requirements to describe the bundled distribution
accurately, removing claims that it has no PyPI release or requires a matching
adapter tag.
In `@python/reflex_xy/events.py`:
- Around line 38-206: Move all numeric semantic-event payloads in
python/reflex_xy/events.py:38-206—including point coordinates, selection
bounds/polygons, row IDs, row projections, and counts—into a defined binary
event ABI, leaving only non-numeric metadata and attachment references in JSON;
update PointData, ScreenPoint, DataBounds, SelectionPayload, and the event
TypedDicts consistently. In python/reflex_xy/selections.py:17-19 and 37-42,
decode the binary selection geometry before calling select_range or
select_polygon, and update resolve_selection() to consume the binary IDs, rows,
and counts while preserving truncation and clear-selection behavior.
In `@python/reflex_xy/namespace.py`:
- Around line 153-167: Add a FigureRegistry operation that atomically snapshots
each entry’s figure and version under the registry mutex, coordinated with
publish. Update on_sub and the other affected handlers to use that snapshot for
payload building and message processing instead of reading entry.figure and
entry.version independently. Before emitting any interaction reply, compare the
current registry version with the captured version and discard the reply when
they differ.
- Around line 132-141: Update on_connect to validate the query-string token
through Reflex’s server-side session manager before calling save_session. Reject
the connection when the token is missing or validation fails, and only store the
validated token for authenticated connections; use the existing
session-validation symbol and connection rejection mechanism already available
in the namespace.
In `@python/reflex_xy/payload_asset.py`:
- Around line 76-82: Update the temporary path construction in the asset-writing
block around dest and tmp so each writer gets a unique filename, using the
process ID plus a random or otherwise collision-resistant suffix. Keep the
existing write_bytes and replace flow, ensuring concurrent writers never share
the same temporary file while publishing the identical final dest.
In `@python/reflex_xy/registry.py`:
- Around line 111-127: Synchronize figure replacement in publish with
entry.lock, using the same generation-safe path as payload construction and
append mutation. Keep figure replacement, version increment, payload or delta
construction, and version capture coordinated so each emitted message references
the exact figure or delta used to build it, preventing stale figures or
incompatible deltas.
In `@python/reflex_xy/state_bridge.py`:
- Around line 63-65: Update the chart conversion flow around _figure_of so its
result is validated as a Figure before publication. If the converted value is
invalid, return None or raise the established controlled builder error; preserve
the existing None handling and only publish valid Figure instances.
In `@scripts/verify_sdist.py`:
- Around line 257-274: Require exactly one valid Reflex requirement for the
reflex extra in scripts/verify_sdist.py:257-274 and
scripts/verify_wheel.py:202-224, rejecting duplicate or conflicting constraints
while preserving the existing minimum-version validation. Add conflicting
second-Reflex-requirement cases in tests/test_verify_sdist.py:231-248 and
tests/test_verify_wheel.py:279-296 to verify both artifact verifiers reject the
metadata.
In `@spec/api/export.md`:
- Line 51: Update the user-facing wording in the export documentation by
replacing “kernel-lessly” with “without a kernel,” while preserving the
surrounding statement and meaning.
In `@spec/process/production-readiness.md`:
- Around line 386-390: Update the wheel verification checklist to apply
scripts/verify_wheel.py --expect-native only to native wheels, and add a
separate fallback-wheel verification using --expect-pure. Keep the release
checks aligned with the native and fallback gates described earlier in the
document.
In `@tests/reflex_adapter/conftest.py`:
- Line 5: Replace the malformed comment text “importorskips” in the Reflex
dependency-rule note with clear wording that directly states these tests are
skipped when the Reflex dependency cannot be imported.
---
Outside diff comments:
In `@spec/design/reflex-shaped-api.md`:
- Around line 556-581: Update the surrounding design section to present the
bundled reflex_xy integration and xy[reflex] extra as the current shipped
implementation, removing stale future-work language and separate-adapter-package
recommendations. Retain smaller dependency surfaces or extracting a separate
package only as explicitly identified future work, and ensure the documented
behavior matches the relevant code, configuration, build, and release metadata.
---
Nitpick comments:
In `@python/reflex_xy/assets/XYChart.jsx`:
- Around line 361-380: Update the static payload fetch flow in the visible
effect around fetch(src) to create an AbortController, pass its signal to fetch,
and abort it during unmount cleanup. Preserve the existing cancelled checks and
ensure abort errors remain safely handled by the current catch path.
In `@python/reflex_xy/component.py`:
- Around line 205-220: Extract lazy _component_cls initialization into a shared
_component() helper, then replace direct _component_cls.create calls with
_component().create in both _facet_grid and chart. Preserve the existing class
construction behavior while ensuring _facet_grid works independently of chart
initialization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 522321d0-e1ab-4f9c-9f27-abaa3e094c27
⛔ Files ignored due to path filters (4)
docs/app/reflex.lock/bun.lockis excluded by!**/*.lockdocs/app/uv.lockis excluded by!**/*.lockpython/reflex-xy/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (84)
.github/workflows/ci.yml.github/workflows/release-reflex-xy.yml.github/workflows/release.ymlCHANGELOG.mdCLAUDE.mdMakefileREADME.mddocs/advanced/runtime-and-deployment.mddocs/api-reference/events-and-callbacks.mddocs/app/pyproject.tomldocs/app/reflex.lock/package.jsondocs/app/rxconfig.pydocs/app/tests/test_docs_site.pydocs/app/xy_docs/api_reference.pydocs/app/xy_docs/demos/benchmark_charts.pydocs/app/xy_docs/demos/xy_sdf_plots.pydocs/app/xy_docs/playground.pydocs/guides/deployment-recipes.mddocs/integrations/index.mddocs/integrations/reflex.mddocs/overview/installation.mdexamples/reflex/README.mdexamples/reflex/assets/.gitkeepexamples/reflex/pyproject.tomlexamples/reflex/rxconfig.pyexamples/reflex/xy_reflex_demo/__init__.pyexamples/reflex/xy_reflex_demo/xy_reflex_demo.pypyproject.tomlpython/reflex-xy/CHANGELOG.mdpython/reflex-xy/README.mdpython/reflex-xy/pyproject.tomlpython/reflex_xy/__init__.pypython/reflex_xy/app.pypython/reflex_xy/assets/XYChart.jsxpython/reflex_xy/assets/__init__.pypython/reflex_xy/component.pypython/reflex_xy/events.pypython/reflex_xy/namespace.pypython/reflex_xy/payload_asset.pypython/reflex_xy/registry.pypython/reflex_xy/selections.pypython/reflex_xy/state_bridge.pypython/reflex_xy/tokens.pypython/reflex_xy/vars.pyscripts/check_release_version.pyscripts/reflex_ws_smoke.pyscripts/verify_ci_workflow.pyscripts/verify_reflex_xy_dist.pyscripts/verify_sdist.pyscripts/verify_wheel.pyspec/README.mdspec/api/chart-roadmap.mdspec/api/export.mdspec/api/interaction.mdspec/design/chart-grammar.mdspec/design/reflex-integration.mdspec/design/reflex-shaped-api.mdspec/design/renderer-architecture.mdspec/design/view-state.mdspec/design/wire-protocol.mdspec/process/contributing.mdspec/process/production-readiness.mdtests/reflex_adapter/__init__.pytests/reflex_adapter/conftest.pytests/reflex_adapter/test_assets.pytests/reflex_adapter/test_async_figure_var.pytests/reflex_adapter/test_component.pytests/reflex_adapter/test_figure_var.pytests/reflex_adapter/test_payload_asset.pytests/reflex_adapter/test_registry.pytests/reflex_adapter/test_selections.pytests/reflex_adapter/test_socket_data_plane.pytests/reflex_adapter/test_state_bridge.pytests/reflex_adapter/test_tokens.pytests/reflex_adapter/test_view_state_push.pytests/test_check_release_version.pytests/test_dependencies.pytests/test_payload_update_rehome.pytests/test_tailwind_root_customization.pytests/test_verify_ci_workflow.pytests/test_verify_local.pytests/test_verify_reflex_xy_dist.pytests/test_verify_sdist.pytests/test_verify_wheel.py
💤 Files with no reviewable changes (8)
- docs/app/reflex.lock/package.json
- .github/workflows/release-reflex-xy.yml
- python/reflex-xy/README.md
- python/reflex-xy/CHANGELOG.md
- python/reflex-xy/pyproject.toml
- scripts/verify_reflex_xy_dist.py
- tests/test_verify_ci_workflow.py
- tests/test_verify_reflex_xy_dist.py
🛑 Comments failed to post (6)
python/reflex_xy/events.py (1)
38-206: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Trace the semantic-event producer, its transport, and the documented ABI. rg -n -C 4 \ 'pointEnvelope|point_hover|point_click|select_end|view_change|JSON\.stringify|data_bounds|polygon|x_domain|y_domain|canonical_row_id' \ python/reflex_xy/assets/XYChart.jsx \ python/reflex_xy/events.py \ python/reflex_xy/selections.py \ spec/design/reflex-integration.mdRepository: reflex-dev/xy
Length of output: 34309
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '%s\n' '--- event transport and serialization ---' rg -n -C 5 \ 'addEvents|onPointHover|onPointClick|onSelectEnd|onViewChange|event-trigger|JSON|arrayBuffer|Float32Array|raw buffer|control plane|data plane' \ python/reflex_xy/assets/XYChart.jsx \ python/reflex_xy \ spec/design/reflex-integration.md \ spec/design/wire-protocol.md printf '%s\n' '--- selection resolver implementation ---' cat -n python/reflex_xy/selections.py printf '%s\n' '--- relevant client sections ---' sed -n '230,275p;440,535p;600,670p' python/reflex_xy/assets/XYChart.jsx printf '%s\n' '--- protocol declarations ---' rg -n -C 8 \ 'semantic event|point_hover|point_click|select_end|view_change|f32|Float32Array|binary' \ spec/design/wire-protocol.md spec/design/reflex-integration.mdRepository: reflex-dev/xy
Length of output: 50372
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '%s\n' '--- component event contract ---' sed -n '15,35p;78,92p' python/reflex_xy/component.py printf '%s\n' '--- selection resolver ---' cat -n python/reflex_xy/selections.py printf '%s\n' '--- client event construction ---' sed -n '241,252p;470,480p;644,665p' python/reflex_xy/assets/XYChart.jsx printf '%s\n' '--- data-plane binary contract ---' sed -n '108,118p' spec/design/reflex-integration.md sed -n '490,515p' spec/design/reflex-integration.md printf '%s\n' '--- read-only invariant check ---' python3 - <<'PY' from pathlib import Path component = Path("python/reflex_xy/component.py").read_text() events = Path("python/reflex_xy/events.py").read_text() client = Path("python/reflex_xy/assets/XYChart.jsx").read_text() selections = Path("python/reflex_xy/selections.py").read_text() design = Path("spec/design/reflex-integration.md").read_text() checks = { "semantic events use normal Reflex event props": "on_point_hover: rx.EventHandler" in component, "event types expose numeric fields": '"data": {"x": float, "y": float}' in events and '"x_domain": [x0, x1]' in events, "client constructs numeric point payloads": "data: { x, y }" in client, "client constructs numeric view payloads": "x_domain: [m.x0, m.x1]" in client, "client constructs numeric selection geometry": "data_bounds: message.bounds" in client, "resolver consumes numeric selection geometry": 'bounds["x0"]' in selections and 'selection["polygon"]' in selections, "design permits JSON semantic events": "semantic events out" in design and "low-frequency, JSON" in design, "design requires binary data columns": "No JSON numbers for data" in design, } for name, result in checks.items(): print(f"{'PASS' if result else 'FAIL'}: {name}") if not all(checks.values()): raise SystemExit(1) PYRepository: reflex-dev/xy
Length of output: 8103
Move numeric semantic-event fields to binary attachments.
The data plane is binary, but semantic events currently send numeric coordinates, bounds, polygons, IDs, rows, and counts through Reflex JSON. This violates the no-JSON-numbers wire requirement. Define a binary event ABI, decode selection geometry before
select_rangeorselect_polygon, and updateevents.pyandresolve_selection()accordingly.📍 Affects 2 files
python/reflex_xy/events.py#L38-L206(this comment)python/reflex_xy/selections.py#L17-L19python/reflex_xy/selections.py#L37-L42🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/reflex_xy/events.py` around lines 38 - 206, Move all numeric semantic-event payloads in python/reflex_xy/events.py:38-206—including point coordinates, selection bounds/polygons, row IDs, row projections, and counts—into a defined binary event ABI, leaving only non-numeric metadata and attachment references in JSON; update PointData, ScreenPoint, DataBounds, SelectionPayload, and the event TypedDicts consistently. In python/reflex_xy/selections.py:17-19 and 37-42, decode the binary selection geometry before calling select_range or select_polygon, and update resolve_selection() to consume the binary IDs, rows, and counts while preserving truncation and clear-selection behavior.Source: Coding guidelines
python/reflex_xy/namespace.py (2)
132-141: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect namespace connection handlers and application authentication hooks. ast-grep outline python/reflex_xy/namespace.py --match 'XYNamespace' --view expanded rg -n -C5 \ 'on_connect|connect.*auth|authenticate|authorization|client_token|save_session|get_session' \ --type=py .Repository: reflex-dev/xy
Length of output: 35143
🏁 Script executed:
#!/bin/bash set -euo pipefail cat -n python/reflex_xy/namespace.py | sed -n '1,340p' printf '\n--- adapter setup and socket registrations ---\n' rg -n -C8 \ 'XYNamespace|AsyncServer|register_namespace|/_xy|socketio|/_event|QUERY_STRING|client_token' \ python tests pyproject.toml uv.lock 2>/dev/null || true printf '\n--- targeted socket tests ---\n' cat -n tests/reflex_adapter/test_socket_data_plane.py | sed -n '1,120p;320,390p'Repository: reflex-dev/xy
Length of output: 50370
🌐 Web query:
python-socketio server namespace on_connect authentication shared Engine.IO connection namespace independently authenticated💡 Result:
In Python-SocketIO, namespaces are logical connections multiplexed over a single physical Engine.IO transport [1][2]. Because each namespace operates as an independent logical connection, you can authenticate them individually [1]. Authentication Strategy The recommended way to authenticate a namespace is via its individual 'connect' event handler [3][1]. When a client attempts to connect to a specific namespace, the server triggers the 'connect' event for that namespace [1]. You can reject the connection by returning False or raising an exception from this handler [3][4][5]. Example Implementation You can define independent 'connect' handlers for different namespaces [1]:
@sio.on('connect', namespace='/chat') def connect_chat(sid, environ, auth): # Perform authentication logic specific to /chat if not authenticate_chat(auth): return False # Success: allow connection@sio.on('connect', namespace='/admin') def connect_admin(sid, environ, auth): # Perform authentication logic specific to /admin if not authenticate_admin(auth): return False # Success: allow connection Key Considerations 1. Authentication Data: Clients can pass an 'auth' dictionary (e.g., tokens) in their 'connect' call [6][5]. This data is passed to the server-side 'connect' handler [5]. 2. Independent Sessions: Each namespace connection is assigned its own unique SID [2][7]. You can use 'sio.session(sid, namespace=...)' to manage session data independently for each namespace [8]. 3. Class-based Namespaces: If using class-based namespaces, define an 'on_connect(self, sid, environ, auth)' method within your class to handle authentication for that specific namespace [1][9]. 4. Connection State: Since namespaces are multiplexed, if the underlying Engine.IO connection fails, all namespace connections will also be disconnected [1][2]. However, rejecting a connection at the namespace level only affects that specific logical path [1][5]. While you can technically perform authentication at the Engine.IO level (e.g., using middleware), doing so at the namespace 'connect' handler provides the most granular control, allowing you to easily handle different authentication requirements per namespace [1][5].Citations:
- 1: https://python-socketio.readthedocs.io/en/stable/server.html
- 2: miguelgrinberg/python-socketio#461
- 3: https://python-socketio.readthedocs.io/en/stable/api%5Fserver.html
- 4: https://python-socketio.readthedocs.io/en/v4/api.html
- 5: https://flask-socketio.readthedocs.io/en/stable/getting_started.html
- 6: https://python-socketio.readthedocs.io/en/latest/api_client.html
- 7: miguelgrinberg/python-socketio#1493
- 8: https://github.com/miguelgrinberg/python-socketio/blob/main/src/socketio/server.py
- 9: https://python-socketio.readthedocs.io/en/latest/api_namespace.html
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' import ast from pathlib import Path path = Path("python/reflex_xy/namespace.py") tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) klass = next(n for n in tree.body if isinstance(n, ast.ClassDef) and n.name == "XYNamespace") methods = {n.name: n for n in klass.body if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef))} connect = methods["on_connect"] connect_calls = [ ast.unparse(node) for node in ast.walk(connect) if isinstance(node, ast.Call) ] connect_returns = [ ast.unparse(node.value) if node.value is not None else None for node in ast.walk(connect) if isinstance(node, ast.Return) ] entry = methods["_entry_for"] entry_session_checks = [ ast.unparse(node) for node in ast.walk(entry) if isinstance(node, ast.Compare) and "client_token" in ast.unparse(node) ] print("on_connect calls:", connect_calls) print("on_connect returns:", connect_returns) print("_entry_for client-token checks:", entry_session_checks) print("namespace registration:", "sio.register_namespace(namespace)" in Path("python/reflex_xy/app.py").read_text()) PY printf '\n--- focused client handshake and auth-related tests ---\n' rg -n -C4 \ 'query: \{ token|connect\(|namespaces|/_event|/_xy|auth=|on_connect|reject|unauthor' \ python/reflex_xy/assets/XYChart.jsx tests/reflex_adapter/test_socket_data_plane.py \ tests/reflex_adapter/test_assets.py python/reflex_xy/app.pyRepository: reflex-dev/xy
Length of output: 33936
🌐 Web query:
Reflex framework socket.io client token authentication server connect handler _event namespace client_token💡 Result:
In the Reflex framework, authentication and connection management are handled through the Socket.IO
/_eventnamespace, which manages client sessions using aclient_token[1][2][3]. Theclient_tokenis a unique UUID generated by the frontend and is sent automatically by the Reflex client during the initial WebSocket connection as a query parameter (?token=...) [4][1][3]. The backend'sEventNamespace(via theTokenManager) maps thisclient_tokento the Socket.IO Session ID (sid) to maintain state consistency [1][5][6]. Key points regarding the connection and authentication flow: 1. Connection Handler: While developers typically use Reflex's high-levelrx.Stateand event decorators, the underlying WebSocket connection is managed byEventNamespace.on_connect[1][7]. You can access session information, including theclient_token, within your state handlers viaself.router.session.client_token[4]. 2. Token Lifecycle: The system relies on theTokenManagerto link tokens to connections [5]. If a client reconnects (e.g., after a network blip), the framework attempts to resolve theclient_tokento an existing session [8][9]. If aclient_tokenis detected as already belonging to an activesid, the system may trigger anew_tokenevent to handle potential duplicate tab scenarios or state conflicts [8][9]. 3. Advanced Management: If you need to monitor connection status for background tasks, you can userx.get_token_manager()(introduced in recent versions) to access lifecycle APIs [5]. These APIs allow you to track when a session or token connects or disconnects:python # Example: Tracking connection lifecycle in a background task from reflex.utils import get_token_manager class State(rx.State): `@rx.background` async def monitor_session(self): tm = get_token_manager() # Wait for specific token to connect await tm.when_token_connects(self.router.session.client_token) # Perform logic...4. Multiplexing: Reflex's architecture allows for custom namespaces to multiplex onto the existing/_eventconnection [2][3]. These custom namespaces inherit the same authentication (via query parameters) and connection lifecycle as the main app event stream, meaning you do not need to reimplement token validation if you are building on the sameengine.ioconnection [2][3]. For direct access to the mapping between tokens and socket IDs, you can referenceapp.event_namespace.token_to_sid[6], though utilizing the officialTokenManagerlifecycle APIs is the recommended approach for stable integration [5].Citations:
- 1: https://deepwiki.com/reflex-dev/reflex/5.3-event-triggers-and-frontend-communication
- 2: https://github.com/reflex-dev/xy/blob/main/python/reflex-xy/reflex_xy/namespace.py
- 3: https://github.com/reflex-dev/xy/blob/main/spec/design/reflex-integration.md
- 4: https://reflex.dev/docs/utility-methods/router-attributes/
- 5: reflex-dev/reflex#6126
- 6: reflex-dev/reflex#3388
- 7: https://github.com/reflex-dev/reflex/blob/3c114519/reflex/app.py
- 8: reflex-dev/reflex#6252
- 9: reflex-dev/reflex#6345
Reject unauthenticated
/_xyconnections.
on_connectstores the query-string token without validating it through Reflex's server-side session manager. Because/_xyhas independent namespace authentication, a client can connect directly with an arbitrary token, and_entry_fortreats a matching embedded token as authorization. Validate the token before saving the session and reject the connection when validation fails.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/reflex_xy/namespace.py` around lines 132 - 141, Update on_connect to validate the query-string token through Reflex’s server-side session manager before calling save_session. Reject the connection when the token is missing or validation fails, and only store the validated token for authenticated connections; use the existing session-validation symbol and connection rejection mechanism already available in the namespace.
153-167: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Synchronize
entry.figureandentry.versionas one snapshot.
entry.lockdoes not synchronize withFigureRegistry.publish.publishcan replaceentry.figureand incremententry.versionwhile these handlers build a payload or process a message.The server can emit an old figure with a new version. It can also emit an interaction reply from a figure that has already been replaced.
Add a registry operation that captures the figure and version under the registry mutex. Use that snapshot throughout each operation. Before sending an interaction reply, discard it if the current version no longer matches the captured version.
Also applies to: 179-217, 246-256, 304-315
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/reflex_xy/namespace.py` around lines 153 - 167, Add a FigureRegistry operation that atomically snapshots each entry’s figure and version under the registry mutex, coordinated with publish. Update on_sub and the other affected handlers to use that snapshot for payload building and message processing instead of reading entry.figure and entry.version independently. Before emitting any interaction reply, compare the current registry version with the captured version and discard the reply when they differ.python/reflex_xy/payload_asset.py (1)
76-82: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use a unique temporary filename per writer.
The temporary path
.{name}.tmpis identical for every process that writes the same digest. The module documentation states that several workers can import the app module and write concurrently.Path.write_bytestruncates the file first, so one writer can truncate the temporary file while another writer is still writing it. Eitherreplacecall can then publish a short or empty.xyffile, which the frontend fetches and fails to decode. Content addressing keeps the final bytes identical, but it does not protect the shared temporary file.Make the temporary name unique per writer, for example with the process id and a random suffix.
🐛 Proposed fix for the temporary-file race
+import os +import secrets if _should_write(): asset_dir = Path.cwd() / "assets" / ASSET_SUBDIR asset_dir.mkdir(parents=True, exist_ok=True) dest = asset_dir / name if not dest.exists(): # Content-addressed, so concurrent writers (multiple workers # importing the app module) produce identical bytes; the rename # keeps a racing reader from ever seeing a partial file. - tmp = asset_dir / f".{name}.tmp" + # The temporary name is per-writer so concurrent writers never + # truncate each other's in-progress file. + tmp = asset_dir / f".{name}.{os.getpid()}.{secrets.token_hex(4)}.tmp" tmp.write_bytes(frame) tmp.replace(dest)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if not dest.exists(): # Content-addressed, so concurrent writers (multiple workers # importing the app module) produce identical bytes; the rename # keeps a racing reader from ever seeing a partial file. # The temporary name is per-writer so concurrent writers never # truncate each other's in-progress file. tmp = asset_dir / f".{name}.{os.getpid()}.{secrets.token_hex(4)}.tmp" tmp.write_bytes(frame) tmp.replace(dest)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/reflex_xy/payload_asset.py` around lines 76 - 82, Update the temporary path construction in the asset-writing block around dest and tmp so each writer gets a unique filename, using the process ID plus a random or otherwise collision-resistant suffix. Keep the existing write_bytes and replace flow, ensuring concurrent writers never share the same temporary file while publishing the identical final dest.python/reflex_xy/registry.py (1)
111-127: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Serialize figure replacement with payload and delta generation.
publishreplacesentry.figureand incrementsentry.versionwithout acquiringentry.lock. Concurrent payload and append paths use that lock.An
on_subtask can build the old figure and then emit the new version. Anappendtask can mutate the old figure, bump the replacement version, and push an incompatible delta.Use one generation-safe synchronization path for replacement, payload construction, mutation, and version capture. Verify that each emitted version identifies the exact figure or delta used to build the message.
Also applies to: 224-235
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/reflex_xy/registry.py` around lines 111 - 127, Synchronize figure replacement in publish with entry.lock, using the same generation-safe path as payload construction and append mutation. Keep figure replacement, version increment, payload or delta construction, and version capture coordinated so each emitted message references the exact figure or delta used to build it, preventing stale figures or incompatible deltas.python/reflex_xy/state_bridge.py (1)
63-65: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Reject invalid figure-builder results before publishing them.
_figure_of(chart)returns arbitrary objects unchanged. A builder that returns a string can therefore enter the registry. The next subscription callsbuild_payload_spliton that string and raisesAttributeError.Validate the converted result as a
Figure. ReturnNoneor raise a controlled builder error when the result is invalid.Proposed fix
-from typing import TYPE_CHECKING, Any, Optional +from typing import Any, Optional + +from xy._figure import Figure -if TYPE_CHECKING: - from xy._figure import Figure - ... - return _figure_of(chart) + figure = _figure_of(chart) + if not isinstance(figure, Figure): + return None + return figure📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if chart is None: return None figure = _figure_of(chart) if not isinstance(figure, Figure): return None return figure🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/reflex_xy/state_bridge.py` around lines 63 - 65, Update the chart conversion flow around _figure_of so its result is validated as a Figure before publication. If the converted value is invalid, return None or raise the established controlled builder error; preserve the existing None handling and only publish valid Figure instances.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 88 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
|
Addressed the remaining review-body feedback in Implemented:
Two suggestions were intentionally not applied:
Verification: |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/artifact_metadata.py`:
- Around line 18-26: The _dependency_satisfies_floor function currently accepts
pre-release or development versions such as 1.24.dev0 as meeting the stable
minimum. Update its requirement validation to parse and compare the version
portion against minimum, or explicitly reject pre-release/development suffixes
before returning true, while preserving package and optional-extra matching.
In `@spec/design/reflex-shaped-api.md`:
- Around line 789-795: Update the stale separate-adapter roadmap item in Section
10 to remove references to a future independent `reflex-xy` distribution,
keeping the specification consistent with the shipped bundled `reflex_xy`
namespace and existing optional `xy[reflex]` packaging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 13b5be2b-7d1a-4a64-a6b5-b1b269d37b40
📒 Files selected for processing (31)
.github/workflows/ci.yml.github/workflows/release.ymlCLAUDE.mdMakefiledocs/integrations/reflex.mdpython/reflex_xy/assets/XYChart.jsxpython/reflex_xy/component.pypython/reflex_xy/namespace.pypython/reflex_xy/payload_asset.pypython/reflex_xy/py.typedpython/reflex_xy/registry.pypython/reflex_xy/state_bridge.pyscripts/artifact_metadata.pyscripts/reflex_ws_smoke.pyscripts/verify_ci_workflow.pyscripts/verify_sdist.pyscripts/verify_wheel.pyspec/api/export.mdspec/design/reflex-integration.mdspec/design/reflex-shaped-api.mdspec/process/production-readiness.mdtests/reflex_adapter/conftest.pytests/reflex_adapter/test_assets.pytests/reflex_adapter/test_payload_asset.pytests/reflex_adapter/test_registry.pytests/reflex_adapter/test_socket_data_plane.pytests/reflex_adapter/test_state_bridge.pytests/reflex_adapter/test_view_state_push.pytests/test_verify_local.pytests/test_verify_sdist.pytests/test_verify_wheel.py
🚧 Files skipped from review as they are similar to previous changes (22)
- .github/workflows/ci.yml
- .github/workflows/release.yml
- CLAUDE.md
- scripts/reflex_ws_smoke.py
- tests/reflex_adapter/conftest.py
- tests/test_verify_local.py
- python/reflex_xy/state_bridge.py
- Makefile
- python/reflex_xy/payload_asset.py
- tests/test_verify_sdist.py
- spec/api/export.md
- python/reflex_xy/component.py
- tests/reflex_adapter/test_view_state_push.py
- tests/reflex_adapter/test_state_bridge.py
- python/reflex_xy/assets/XYChart.jsx
- tests/test_verify_wheel.py
- python/reflex_xy/namespace.py
- scripts/verify_wheel.py
- spec/design/reflex-integration.md
- scripts/verify_ci_workflow.py
- scripts/verify_sdist.py
- spec/process/production-readiness.md
There was a problem hiding this comment.
All reported issues were addressed across 31 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
python/reflex_xy/assets/XYChart.jsx (1)
420-428: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated interaction-bookkeeping reset into a helper.
clickInputs.clear(); restoreSelectionSeqs.clear();appears identically insubscribe(Line 425-426), the payload version-change branch (Lines 588-589), and the unaddressed-append version-bump branch (Lines 716-717). Extract a small helper, for exampleresetInteractionBookkeeping(), and call it from all three sites.♻️ Proposed refactor
+ const resetInteractionBookkeeping = () => { + clickInputs.clear(); + restoreSelectionSeqs.clear(); + }; + const subscribe = () => { payloadVersion = null; awaitingPayload = true; - clickInputs.clear(); - restoreSelectionSeqs.clear(); + resetInteractionBookkeeping(); socket.emit("sub", { fig: token, px: el.clientWidth || null, mid }); };if (payloadVersion !== null && nextPayloadVersion !== payloadVersion) { - clickInputs.clear(); - restoreSelectionSeqs.clear(); + resetInteractionBookkeeping(); }if (wireVersion !== null && message.type === "append" && data.mid == null) { - clickInputs.clear(); - restoreSelectionSeqs.clear(); + resetInteractionBookkeeping(); payloadVersion = wireVersion; }Also applies to: 585-590, 715-719
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/reflex_xy/assets/XYChart.jsx` around lines 420 - 428, Extract the duplicated clickInputs and restoreSelectionSeqs clearing logic into a resetInteractionBookkeeping helper, then call that helper from subscribe, the payload version-change branch, and the unaddressed-append version-bump branch. Preserve the existing reset timing and surrounding version-handling behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/reflex_xy/registry.py`:
- Around line 71-74: Bound the lifetime of entries created in Registry.sweep()
by storing each evicted version with its eviction timestamp, then prune entries
older than a defined multiple of the configured TTL during subsequent sweeps.
Update publish() and release() to continue removing the corresponding records
while preserving the scalar version used for state-driven rebuilds.
---
Nitpick comments:
In `@python/reflex_xy/assets/XYChart.jsx`:
- Around line 420-428: Extract the duplicated clickInputs and
restoreSelectionSeqs clearing logic into a resetInteractionBookkeeping helper,
then call that helper from subscribe, the payload version-change branch, and the
unaddressed-append version-bump branch. Preserve the existing reset timing and
surrounding version-handling behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 615424e9-b16b-4e51-81c3-71b68c7da6ce
📒 Files selected for processing (14)
.github/workflows/ci.ymlpython/reflex_xy/assets/XYChart.jsxpython/reflex_xy/namespace.pypython/reflex_xy/registry.pyscripts/artifact_metadata.pyscripts/verify_ci_workflow.pyspec/design/reflex-integration.mdspec/design/reflex-shaped-api.mdtests/reflex_adapter/test_assets.pytests/reflex_adapter/test_registry.pytests/reflex_adapter/test_socket_data_plane.pytests/test_verify_ci_workflow.pytests/test_verify_sdist.pytests/test_verify_wheel.py
🚧 Files skipped from review as they are similar to previous changes (6)
- .github/workflows/ci.yml
- scripts/artifact_metadata.py
- tests/reflex_adapter/test_assets.py
- spec/design/reflex-shaped-api.md
- scripts/verify_ci_workflow.py
- tests/test_verify_wheel.py
There was a problem hiding this comment.
All reported issues were addressed across 14 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 14 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 13 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
python/reflex_xy/namespace.py (1)
443-446: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRecord the builder failure before you discard it.
The
except Exceptionbranch replaces any builder error withfigure = None. The client then receivesunknown figure token, and the operator gets no signal about the real cause. Add a debug or warning log with the token and exception so rebuild failures stay diagnosable.♻️ Proposed logging addition
try: figure = await rebuild(token) - except Exception: # noqa: BLE001 - user builder code is an input boundary + except Exception: # noqa: BLE001 - user builder code is an input boundary + _LOGGER.warning("figure rebuild failed for %s", token, exc_info=True) figure = NoneDefine the module logger once near the imports:
import logging _LOGGER = logging.getLogger(__name__)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/reflex_xy/namespace.py` around lines 443 - 446, Update the rebuild exception handler around rebuild to log the failed token and caught exception before setting figure to None. Define and reuse the module-level _LOGGER near the imports, using debug or warning-level logging without changing the existing fallback behavior.tests/test_verify_ci_workflow.py (1)
57-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRead
ci.ymlthrough a repository-root helper.Every new test resolves
Path(".github/workflows/ci.yml")relative to the current working directory. The suite then only passes when pytest runs from the repository root, while_load_verify_module(Line 24) already anchors on__file__. Extract one module-level helper and reuse it; that also removes the repeated read in thirteen tests.♻️ Proposed helper
REPO_ROOT = Path(__file__).resolve().parents[1] def _ci_workflow_text() -> str: return (REPO_ROOT / ".github" / "workflows" / "ci.yml").read_text(encoding="utf-8")def test_locked_reflex_environment_must_be_in_named_install_step(tmp_path: Path) -> None: - workflow = Path(".github/workflows/ci.yml").read_text(encoding="utf-8") + workflow = _ci_workflow_text() required = " uv sync --locked --extra reflex --group dev"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_verify_ci_workflow.py` around lines 57 - 59, Define a module-level repository-root constant and _ci_workflow_text() helper in tests/test_verify_ci_workflow.py, anchored from __file__ like _load_verify_module. Replace all direct Path(".github/workflows/ci.yml").read_text(...) calls across the tests with this helper, including the shown test, so workflow loading is independent of pytest’s working directory and the repeated reads are centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spec/design/reflex-integration.md`:
- Around line 145-153: Update the envelope table in the reflex integration
specification so its mid notation matches the optional implementation contract:
mark mid optional for msg and remove it from unsub, which does not consume the
field. Keep the existing payload and message field names unchanged.
---
Nitpick comments:
In `@python/reflex_xy/namespace.py`:
- Around line 443-446: Update the rebuild exception handler around rebuild to
log the failed token and caught exception before setting figure to None. Define
and reuse the module-level _LOGGER near the imports, using debug or
warning-level logging without changing the existing fallback behavior.
In `@tests/test_verify_ci_workflow.py`:
- Around line 57-59: Define a module-level repository-root constant and
_ci_workflow_text() helper in tests/test_verify_ci_workflow.py, anchored from
__file__ like _load_verify_module. Replace all direct
Path(".github/workflows/ci.yml").read_text(...) calls across the tests with this
helper, including the shown test, so workflow loading is independent of pytest’s
working directory and the repeated reads are centralized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4c481e96-0a93-4e9f-8a8f-e50e8b0fb98a
📒 Files selected for processing (18)
README.mdpyproject.tomlpython/reflex_xy/assets/XYChart.jsxpython/reflex_xy/namespace.pypython/reflex_xy/registry.pyscripts/artifact_metadata.pyscripts/verify_ci_workflow.pyscripts/verify_sdist.pyspec/design/reflex-integration.mdspec/process/production-readiness.mdspec/process/rendering-verification.mdtests/reflex_adapter/test_assets.pytests/reflex_adapter/test_registry.pytests/reflex_adapter/test_socket_data_plane.pytests/test_tailwind_root_customization.pytests/test_verify_ci_workflow.pytests/test_verify_sdist.pytests/test_verify_wheel.py
🚧 Files skipped from review as they are similar to previous changes (10)
- tests/test_tailwind_root_customization.py
- scripts/verify_sdist.py
- tests/reflex_adapter/test_assets.py
- scripts/artifact_metadata.py
- pyproject.toml
- spec/process/production-readiness.md
- tests/test_verify_sdist.py
- README.md
- python/reflex_xy/assets/XYChart.jsx
- tests/test_verify_wheel.py
There was a problem hiding this comment.
All reported issues were addressed across 12 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
1 participant
Summary
reflex_xyadapter source into the mainxydistributionreflexextra withreflex>=0.9.6, while keeping plainxyframework-freereflex-xypackage metadata, lockfile, release workflow, version gate, and distribution verifierxy[reflex]Why
The Reflex integration is owned and maintained with XY, so publishing it as a separate distribution creates an unnecessary third-party-looking package boundary and allows the adapter, browser client, and core library to drift. Shipping the adapter in every
xywheel keeps those pieces version-coherent; the extra only selects the supported Reflex dependency floor.User impact
Reflex users install:
The Python import remains
import reflex_xy. Existing users no longer need a separately versionedreflex-xydistribution. Users who install plainxydo not acquire Reflex.Validation
3760 passed, 108 skipped103 passed, 1 xfailedxywheel with[reflex]: matchingxy/reflex_xyversions, noreflex-xydistribution, successful production compile, one mounted live chart, and nonblank rendered pixelsruff check .,ruff format --check ., workflow verification, andgit diff --checkSummary by CodeRabbit
xy[reflex]installation option with compatible Reflex dependency selection.