From 9fad699c7244a7d72a91855484a99d68826dae28 Mon Sep 17 00:00:00 2001 From: Farhan Ali Raza <62690310+FarhanAliRaza@users.noreply.github.com> Date: Wed, 5 Aug 2026 19:16:41 +0500 Subject: [PATCH 1/6] feat(reflex): data-bound chart factories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The user-facing half of the tier: reflex_xy.scatter_chart(data=Dash.cloud, x="x", y="y", color="mag") and the composed reflex_xy.chart(*nodes, data=...) for multi-mark charts. A factory call at page evaluation builds a plan, so the chart's structure is validated at `reflex run` rather than at hydrate. What that buys, in the order a user hits it: a hallucinated factory name fails at import (the xy node re-exports are an explicit curated map, not getattr passthrough); an unknown kwarg fails with a did-you-mean; a bad colormap, enum, or axis ref fails in the zero-row probe; an unknown column name fails against the data var's TypedDict without executing the data method; and the wrong var or a raw string in data= fails on the typed prop. The kwarg partition — mark options vs chrome vs component props vs event handlers — is derived from inspect.signature at import rather than hand-listed, because a hand-listed partition silently drifts from xy's signatures and Reflex absorbs unknown non-event kwargs into `style` where a typo would vanish rather than raise (the hazard Phase 0 pinned). Collisions get generated aliases, pinned by test. Two mounts from one surface: a Var data source becomes the plan/data props the wrapper composes into a composite subscription, while a concrete mapping binds immediately and routes to the static payload-asset path — same validation, works under `reflex export`. Aggregating kinds (box, violin, hexbin, contour, heatmap, stairs, ecdf) and the data-taking composites (pie, radar, wind_rose, sankey) are excluded from the plan tier and refused by name with the two working routes: their validators need real values, and a synthetic-row probe would validate against made-up data — a silent decimation of the compile guarantee. Page-plan registration lands here too, with the factories that populate the map: backend-only workers import the app without evaluating pages, so the startup lifespan evaluates them once, making "the plan map is populated in every worker" true by construction instead of an assumption about Reflex. Spec: reflex-integration.md §3.6 (factories, static symmetry, kind coverage, page-plan registration), file map. --- python/reflex_xy/__init__.py | 178 ++++++- python/reflex_xy/app.py | 44 +- python/reflex_xy/assets/XYChart.jsx | 32 +- python/reflex_xy/component.py | 11 +- python/reflex_xy/factories.py | 489 ++++++++++++++++++ spec/design/reflex-integration.md | 59 ++- tests/reflex_adapter/test_factories.py | 320 ++++++++++++ .../test_page_plan_registration.py | 74 +++ 8 files changed, 1175 insertions(+), 32 deletions(-) create mode 100644 python/reflex_xy/factories.py create mode 100644 tests/reflex_adapter/test_factories.py create mode 100644 tests/reflex_adapter/test_page_plan_registration.py diff --git a/python/reflex_xy/__init__.py b/python/reflex_xy/__init__.py index 7e184303..0f012acf 100644 --- a/python/reflex_xy/__init__.py +++ b/python/reflex_xy/__init__.py @@ -3,38 +3,55 @@ The integration in one paragraph (full design: spec/design/reflex-integration.md in the xy repo): chart data rides the app's *existing* websocket as a second socket.io namespace — binary -columns, no JSON numbers, no extra endpoints to proxy. Figures live in a -per-process registry keyed by tokens; Reflex state holds only a small typed -handle wrapping the token. A `@reflex_xy.figure` state method is both the -chart definition and the recovery recipe: any worker can rebuild the figure -from state when a reconnect lands somewhere new, so there is no central -figure store to operate. +columns, no JSON numbers, no extra endpoints to proxy. Figures and columns +live in a per-process registry keyed by tokens; Reflex state holds only +small typed handles. State methods are both the definition and the +recovery recipe: any worker can rebuild what it serves from state when a +reconnect lands somewhere new, so there is no central figure store to +operate. -Quickstart:: +Quickstart (the data-bound component API — structure is declared in the +page and validated at ``reflex run``; state supplies only columns):: # rxconfig.py config = rx.Config(app_name="dash", plugins=[reflex_xy.XYPlugin()]) # dash/dash.py + from typing import TypedDict import numpy as np import reflex as rx - import xy import reflex_xy + class CloudData(TypedDict): + x: np.ndarray + y: np.ndarray + mag: np.ndarray + class Dash(rx.State): points: int = 200_000 - @reflex_xy.figure - def chart(self) -> xy.Chart: + @reflex_xy.data + def cloud(self) -> CloudData: rng = np.random.default_rng(7) - xs = rng.normal(size=self.points) - ys = xs * 0.6 + rng.normal(scale=0.6, size=self.points) - return xy.scatter_chart(xy.scatter(xs, ys), width="100%", height=460) + x = rng.normal(size=self.points) + y = x * 0.6 + rng.normal(scale=0.6, size=self.points) + return {"x": x, "y": y, "mag": np.hypot(x, y)} def index() -> rx.Component: - return reflex_xy.chart(figure=Dash.chart, height="460px") + return reflex_xy.scatter_chart( + data=Dash.cloud, + x="x", y="y", color="mag", colormap="viridis", + height="460px", + ) app = rx.App() + +Multi-mark charts compose xy nodes around the same data var +(``reflex_xy.chart(reflex_xy.scatter("x", "y"), reflex_xy.line("x", "t"), +data=Dash.cloud)``), and charts whose *structure* depends on state keep the +escape hatch: an ``@reflex_xy.figure`` method returning an ``xy.Chart``, +rendered with ``reflex_xy.chart(figure=Dash.built)`` and probed at compile +(§3.1 of the design). """ from __future__ import annotations @@ -53,7 +70,18 @@ def index() -> rx.Component: "select": ".app", "set_view": ".app", "setup": ".app", - "chart": ".component", + "chart": ".factories", + "area_chart": ".factories", + "bar_chart": ".factories", + "column_chart": ".factories", + "error_band_chart": ".factories", + "errorbar_chart": ".factories", + "histogram_chart": ".factories", + "line_chart": ".factories", + "scatter_chart": ".factories", + "segments_chart": ".factories", + "stem_chart": ".factories", + "step_chart": ".factories", "AsyncDataVar": ".data_vars", "DataVar": ".data_vars", "data": ".data_vars", @@ -79,6 +107,65 @@ def index() -> rx.Component: "figure": ".vars", } +#: Curated re-exports of xy node constructors: `reflex_xy.scatter` *is* +#: `xy.scatter`, so composed data-bound charts read uniformly +#: (`reflex_xy.chart(reflex_xy.scatter("x", "y"), data=...)`) and a +#: hallucinated constructor dies at import against this explicit map +#: instead of surviving to hydrate. Marks whose validators need data +#: (box, violin, hexbin, …) are still listed — the plan probe refuses them +#: with the recorded Phase 3 guidance rather than a misleading +#: AttributeError. Chart factories are deliberately absent: the flat +#: `*_chart` names above are reflex-native factories, not xy's. +_XY_REEXPORTS = frozenset( + { + # marks + "scatter", + "line", + "area", + "step", + "stairs", + "stem", + "column", + "bar", + "histogram", + "errorbar", + "error_band", + "segments", + "ecdf", + "box", + "violin", + "hexbin", + "contour", + "heatmap", + # annotations + "vline", + "hline", + "x_band", + "y_band", + "text", + "label", + "marker", + "arrow", + "threshold", + "threshold_zone", + "callout", + # chrome + config constructors + "x_axis", + "y_axis", + "theta_axis", + "r_axis", + "legend", + "tooltip", + "colorbar", + "modebar", + "export_config", + "theme", + "interaction_config", + "animation", + "spring", + } +) + __all__ = [ "XY_NAMESPACE", "AsyncDataVar", @@ -100,26 +187,85 @@ def index() -> rx.Component: "ViewChangeEvent", "XYNamespace", "XYPlugin", + "animation", "append", + "area", + "area_chart", + "arrow", + "bar", + "bar_chart", + "box", + "callout", "chart", "clear_selection", + "colorbar", + "column", + "column_chart", + "contour", "data", + "ecdf", + "error_band", + "error_band_chart", + "errorbar", + "errorbar_chart", + "export_config", "figure", + "heatmap", + "hexbin", + "histogram", + "histogram_chart", + "hline", "inline", + "interaction_config", + "label", + "legend", + "line", + "line_chart", + "marker", + "modebar", + "r_axis", "register", "registry", "release", "reset_view", "resolve_selection", + "scatter", + "scatter_chart", + "segments", + "segments_chart", "select", "set_view", "setup", + "spring", + "stairs", + "stem", + "stem_chart", + "step", + "step_chart", + "text", + "theme", + "theta_axis", + "threshold", + "threshold_zone", + "tooltip", + "violin", + "vline", + "x_axis", + "x_band", + "y_axis", + "y_band", ] def _load_export(name: str) -> Any: module_name = _EXPORTS.get(name) if module_name is None: + if name in _XY_REEXPORTS: + import xy + + value = getattr(xy, name) + globals()[name] = value + return value raise AttributeError(f"module {__name__!r} has no attribute {name!r}") value = getattr(import_module(module_name, __name__), name) @@ -236,7 +382,6 @@ def release(token: "str | FigureHandle") -> None: if TYPE_CHECKING: from .app import XYPlugin, append, clear_selection, reset_view, select, set_view, setup - from .component import chart from .data_vars import AsyncDataVar, DataVar, data from .events import ( CanonicalRowIdGroup, @@ -250,6 +395,7 @@ def release(token: "str | FigureHandle") -> None: SelectionPayload, ViewChangeEvent, ) + from .factories import bar_chart, chart, histogram_chart, line_chart, scatter_chart from .handles import DataHandle, FigureHandle from .namespace import XY_NAMESPACE, XYNamespace from .registry import FigureRegistry, registry diff --git a/python/reflex_xy/app.py b/python/reflex_xy/app.py index 8d285a64..eae7d30e 100644 --- a/python/reflex_xy/app.py +++ b/python/reflex_xy/app.py @@ -21,6 +21,7 @@ import asyncio import contextlib +import warnings from typing import Any, Optional from reflex.plugins import Plugin @@ -58,11 +59,52 @@ def setup(app: Any) -> XYNamespace: namespace = XYNamespace(registry, rebuild=make_rebuild_hook(app)) sio.register_namespace(namespace) wire(namespace) - app.register_lifespan_task(_xy_lifespan) + + async def _lifespan() -> None: + # The lifespan runs at server startup: every page has been added and + # the serving loop exists, in *every* worker — including backend-only + # workers that never ran the frontend compile. Register the plans + # there before serving (see _ensure_page_plans), then run the sweep. + _ensure_page_plans(app) + await _xy_lifespan() + + app.register_lifespan_task(_lifespan) _namespace = namespace return namespace +def _ensure_page_plans(app: Any) -> None: + """Evaluate the app's page component functions so chart plans register. + + The data-bound tier's plan map is process-local and populated by the + chart factories *as page bodies run* (reflex-integration.md §3.6). A + backend-only worker — dev backend subprocesses and prod workers alike — + imports the app module but skips the frontend compile, so its pages sit + unevaluated and every plan subscription would answer `err {resync}` + forever. Running the page functions here makes "the plan map is + populated in every worker" true by construction; the built component + trees are discarded (plans and payload assets are content-addressed and + idempotent). A failing page degrades to a warning: it would have failed + the real compile in the compile process, and one broken page must not + take down the data plane for the others. + """ + pages = getattr(app, "_unevaluated_pages", None) or {} + for route, page in dict(pages).items(): + component = getattr(page, "component", None) + if not callable(component): + continue # already-built component instances registered at add_page + try: + component() + except Exception as exc: # noqa: BLE001 - user page code is an input boundary + warnings.warn( + f"reflex_xy: evaluating page {route!r} for chart-plan " + f"registration failed: {exc!r}. Data-bound charts declared on " + "this page cannot be served by this worker.", + RuntimeWarning, + stacklevel=2, + ) + + def wire(namespace: XYNamespace) -> None: """Point the registry's fan-out seams at a namespace (setup and tests).""" registry.on_publish(namespace.broadcast_payload) diff --git a/python/reflex_xy/assets/XYChart.jsx b/python/reflex_xy/assets/XYChart.jsx index 91feaa90..c5d24697 100644 --- a/python/reflex_xy/assets/XYChart.jsx +++ b/python/reflex_xy/assets/XYChart.jsx @@ -1,8 +1,10 @@ // XYChart: mount a xy figure inside a Reflex app. // -// Two modes (spec/design/reflex-integration.md). A live subscription has -// two spellings that reduce to one token: `figure` ({token} — the typed -// FigureHandle) and the deprecated bare `token` string. +// Two modes (spec/design/reflex-integration.md). Live subscriptions have +// three spellings that reduce to one token: `figure` ({token} — the typed +// FigureHandle), the deprecated bare `token` string, and the plan tier's +// `plan` digest + `data` ({token} DataHandle), composed client-side as +// `xyp1||` once the data handle hydrates. // // Live — this component does NOT open its own connection. // socket.io multiplexing reuses the app's engine.io websocket when the @@ -262,6 +264,8 @@ export function XYChart(props) { const { token, figure, + plan, + data, src, onPointHover, onPointClick, @@ -279,11 +283,16 @@ export function XYChart(props) { ...divProps } = props; void _tailwindClassTokens; - // One subscription token from the two live spellings. `figure` is the + // One subscription token from the three live spellings. `figure` is the // typed handle ({token}); the bare `token` string is the deprecated wire. // A present handle always wins — its empty token means "not ready" (no - // subscription yet), never a fallback to the legacy spelling. - const liveToken = figure != null ? figure.token || null : token || null; + // subscription yet), never a fallback to the legacy spelling. A plan + // digest + data handle compose the plan-tier token client-side, again + // only once the data handle is hydrated. + const figureToken = figure != null ? figure.token || null : token || null; + const liveToken = (plan && data && data.token) + ? `xyp1|${plan}|${data.token}` + : figureToken; const elRef = useRef(null); // inner chart mount (wiped on payload swaps) const outerRef = useRef(null); // stable wrapper: events, tooltip slot const tooltipSlotRef = useRef(null); @@ -428,6 +437,11 @@ export function XYChart(props) { const viewCallbacks = []; const pendingStatePushes = []; let awaitingPayload = true; + // Consecutive server-initiated resyncs without an intervening payload. + // Bounds the retry loop when a resync can never succeed (e.g. a stale + // plan digest after hot-reload drift; the remount with the new digest + // resubscribes on its own). + let errResyncs = 0; const resetEpoch = () => { // Timer callbacks capture row/domain data from the preceding figure @@ -706,6 +720,7 @@ export function XYChart(props) { && payloadVersion !== null && data.version < payloadVersion ) return; + errResyncs = 0; // an applied payload proves resubscription works again const nextPayloadVersion = Number.isInteger(data.version) ? data.version : null; const sameGenerationAddressedReplacement = data.mid != null && @@ -900,7 +915,10 @@ export function XYChart(props) { const onErr = (data) => { if (destroyed || !data || data.fig !== liveToken) return; console.warn(`xy: ${data.error} (fig ${data.fig})`); - if (data.resync === true && socket.connected) subscribe(); + if (data.resync === true && socket.connected && errResyncs < 5) { + errResyncs += 1; + subscribe(); + } }; const onDisconnect = () => { diff --git a/python/reflex_xy/component.py b/python/reflex_xy/component.py index 2655c174..8d04af21 100644 --- a/python/reflex_xy/component.py +++ b/python/reflex_xy/component.py @@ -49,7 +49,7 @@ from xy.facets import FacetGrid from .assets import WRAPPER_TAG, register -from .handles import FigureHandle +from .handles import DataHandle, FigureHandle from .payload_asset import payload_asset from .registry import _figure_of @@ -85,6 +85,11 @@ class XYChart(rx.Component): # Deprecated live mode: the bare token string. Kept for one release # cycle; the wrapper accepts both (figure wins). token: rx.Var[str] + # Data-bound mode (plan tier): a compile-validated chart plan digest + # plus the DataHandle var whose columns it binds. The wrapper + # composes the ``xyp1||`` subscription itself. + plan: rx.Var[str] + data: rx.Var[DataHandle] # Static mode: URL of a payload asset (XYBF frame) to render # kernel-less. src: rx.Var[str] @@ -139,8 +144,8 @@ def create(cls, *children: Any, **props: Any) -> Any: f"{', '.join(offenders)} need the interaction kernel and never " "fire on a static chart source (a Chart/Figure compiled to a " "payload asset). Serve the figure live instead — a " - "@reflex_xy.figure state var or register()/inline() — " - "or drop the handler(s). Client-side " + "@reflex_xy.figure state var, a data-bound factory chart, or " + "register()/inline() — or drop the handler(s). Client-side " "events (on_hover, on_view_change) work on static charts." ) raise ValueError(msg) diff --git a/python/reflex_xy/factories.py b/python/reflex_xy/factories.py new file mode 100644 index 00000000..9e1e15e9 --- /dev/null +++ b/python/reflex_xy/factories.py @@ -0,0 +1,489 @@ +"""Data-bound chart factories: the component-shaped `reflex_xy` chart API. + +The flat, single-mark form (Level 1 of +spec/design/reflex-component-api-options.md §5.6):: + + reflex_xy.scatter_chart( + data=Dash.cloud, # @reflex_xy.data var (columns only) + x="x", y="y", color="mag", # channels: column-name strings + colormap="viridis", # mark options, validated at compile + x_axis=reflex_xy.x_axis(label="σ"), + height="460px", + on_select_end=Dash.select, + ) + +and the composed, multi-mark form (Level 2):: + + reflex_xy.chart( + reflex_xy.scatter(x="x", y="y", color="mag"), + reflex_xy.line(x="x", y="trend", width=2), + reflex_xy.x_axis(label="Time"), + data=Dash.cloud, + height="460px", + ) + +Both compile the real xy tree at page evaluation, run the zero-row probe +(`plan.py` — the full mark/config validation gate, X1/X2), check channel +names against the data var's TypedDict schema (R7), and mount the private +component with a `plan` digest plus the `data` handle var. Marks and chrome +are plain xy dataclass nodes — they never enter the Reflex tree +(Reflex child validation would refuse them; design decision 1 in the +options doc §5.6), which is why `chart(...)` here is a factory consuming +nodes, not a component taking children. + +The kwarg partition of the flat form is **derived from signatures**, not +hand-listed: positional mark params are channels, keyword-only params are +options (xy's uniform convention), `Chart.__init__` provides the +chart-level names, and a small reserved set belongs to the component. +Colliding mark options get flat aliases (`stroke_width` for `line`'s +`width`, `mark_` otherwise); the resulting table is pinned by +`tests/reflex_adapter/test_factories.py` as public contract. Unknown +kwargs near a known name error with a suggestion; anything else falls +through to the component as CSS, preserving Reflex's style convention +(the R8 hazard this partition exists to compensate for). +""" + +from __future__ import annotations + +import difflib +import inspect +from collections.abc import Mapping +from dataclasses import dataclass +from typing import Any, Optional, get_args, get_origin, get_type_hints, is_typeddict + +import reflex as rx + +import xy as _xy +from xy.components import Axis, Chart, Component, Legend, Theme + +from .component import ( + _component, + _merge_tailwind_class_tokens, + _tailwind_class_manifest, + _tailwind_class_tokens, + _tailwind_scan_literal, +) +from .data_vars import validate_columns +from .handles import DataHandle +from .payload_asset import payload_asset +from .plan import ChartPlan, build_plan + +__all__ = [ + "area_chart", + "bar_chart", + "chart", + "column_chart", + "error_band_chart", + "errorbar_chart", + "histogram_chart", + "line_chart", + "scatter_chart", + "segments_chart", + "step_chart", +] + +#: Event triggers of the private XYChart component. Pinned against the +#: component class by tests so the two can never drift. +EVENT_TRIGGERS = frozenset( + { + "on_point_hover", + "on_point_click", + "on_select_end", + "on_view_change", + "on_hover", + "on_animation_start", + "on_animation_end", + } +) + +#: Names the component level owns in the flat form (the collision table's +#: "component/chart level wins" rule). A mark option with one of these names +#: is reachable through its generated alias instead. +COMPONENT_RESERVED = frozenset( + { + "width", + "height", + "opacity", + "style", + "class_name", + "key", + "animation", + "id", + "data", + "figure", + "token", + "plan", + "src", + "tooltip", + "tailwind_classes", + } +) + +#: Chrome slots the flat form accepts as kwargs. x_axis/y_axis are +#: type-dispatched: an Axis node is chrome, a string is the mark's axis id. +_CHROME_KWARGS = ("x_axis", "y_axis", "legend", "theme") + +#: Chart.__init__ params that never partition to the chart tier: identity / +#: children / data are factory-owned; width/height/class_name/style collide +#: with the component (which wins); the on_* callables are the notebook +#: callback API — in Reflex, on_* means event-trigger props. +_CHART_EXCLUDED = frozenset( + { + "self", + "kind", + "children", + "data", + "width", + "height", + "class_name", + "style", + "on_hover", + "on_click", + "on_brush", + "on_select", + "on_view_change", + } +) + +CHART_PARAMS = frozenset( + name for name in inspect.signature(Chart.__init__).parameters if name not in _CHART_EXCLUDED +) + + +def _mark_aliases(mark_params: frozenset[str]) -> dict[str, str]: + """Generated flat aliases for mark options shadowed by the component. + + ``width`` prefers the natural ``stroke_width`` when the mark hasn't + already claimed it (line's stroke width *is* its width); everything else + is uniformly ``mark_``. + """ + aliases: dict[str, str] = {} + for name in sorted(mark_params & COMPONENT_RESERVED): + if name == "width" and "stroke_width" not in mark_params: + aliases["stroke_width"] = name + else: + aliases[f"mark_{name}"] = name + return aliases + + +@dataclass(frozen=True) +class _FlatKind: + """One flat factory's derived partition.""" + + chart_kind: str + mark_factory: Any + mark_params: frozenset[str] # real mark kwargs (data excluded) + aliases: dict[str, str] # flat alias -> real mark param + + @property + def flat_mark_params(self) -> frozenset[str]: + plain = self.mark_params - COMPONENT_RESERVED - {"x_axis", "y_axis"} + return plain | frozenset(self.aliases) + + @property + def known(self) -> frozenset[str]: + return ( + self.flat_mark_params + | CHART_PARAMS + | COMPONENT_RESERVED + | frozenset(_CHROME_KWARGS) + | EVENT_TRIGGERS + ) + + +def _flat_kind(chart_kind: str, mark_factory: Any) -> _FlatKind: + params = frozenset(inspect.signature(mark_factory).parameters) - {"data"} + return _FlatKind( + chart_kind=chart_kind, + mark_factory=mark_factory, + mark_params=params, + aliases=_mark_aliases(params), + ) + + +FLAT_KINDS: dict[str, _FlatKind] = { + kind.chart_kind: kind + for kind in ( + _flat_kind("scatter_chart", _xy.scatter), + _flat_kind("line_chart", _xy.line), + _flat_kind("histogram_chart", _xy.histogram), + _flat_kind("bar_chart", _xy.bar), + _flat_kind("area_chart", _xy.area), + _flat_kind("step_chart", _xy.step), + _flat_kind("stem_chart", _xy.stem), + _flat_kind("column_chart", _xy.column), + _flat_kind("errorbar_chart", _xy.errorbar), + _flat_kind("error_band_chart", _xy.error_band), + _flat_kind("segments_chart", _xy.segments), + ) +} + + +def _suggest(name: str, candidates: frozenset[str]) -> Optional[str]: + matches = difflib.get_close_matches(name, sorted(candidates), n=1, cutoff=0.8) + return matches[0] if matches else None + + +def _partition_flat( + kind: _FlatKind, kwargs: dict[str, Any] +) -> tuple[dict[str, Any], dict[str, Any], list[Component], dict[str, Any]]: + """Split flat kwargs into (mark, chart, chrome children, component).""" + mark_kwargs: dict[str, Any] = {} + chart_kwargs: dict[str, Any] = {} + chrome: list[Component] = [] + component_kwargs: dict[str, Any] = {} + for name, value in kwargs.items(): + if name in ("x_axis", "y_axis"): + # Type-dispatched collision: an Axis node is a chrome child, a + # string is the mark's axis-id option. + if isinstance(value, Axis): + chrome.append(value) + elif isinstance(value, str): + mark_kwargs[name] = value + else: + msg = ( + f"{kind.chart_kind}() {name}= takes an axis node " + f"(reflex_xy.{name}(...)) or an axis-id string, got {type(value).__name__}" + ) + raise TypeError(msg) + elif name == "legend": + if isinstance(value, Legend): + chrome.append(value) + elif isinstance(value, bool): + chrome.append(_xy.legend(show=value)) + else: + msg = ( + f"{kind.chart_kind}() legend= takes reflex_xy.legend(...) or a bool, " + f"got {type(value).__name__}" + ) + raise TypeError(msg) + elif name == "theme": + if not isinstance(value, Theme): + msg = ( + f"{kind.chart_kind}() theme= takes reflex_xy.theme(...), " + f"got {type(value).__name__}" + ) + raise TypeError(msg) + chrome.append(value) + elif name in EVENT_TRIGGERS or name in COMPONENT_RESERVED: + component_kwargs[name] = value + elif name in kind.aliases: + mark_kwargs[kind.aliases[name]] = value + elif name in kind.mark_params: + mark_kwargs[name] = value + elif name in CHART_PARAMS: + chart_kwargs[name] = value + elif name.startswith("on_"): + # Let the framework's create() raise its ValueError listing the + # valid triggers (R8) — unless a near-miss lets us say better. + suggestion = _suggest(name, EVENT_TRIGGERS) + if suggestion is not None: + msg = ( + f"{kind.chart_kind}() got an unexpected event {name!r}; " + f"did you mean {suggestion!r}?" + ) + raise TypeError(msg) + component_kwargs[name] = value + else: + suggestion = _suggest(name, kind.known) + if suggestion is not None: + msg = ( + f"{kind.chart_kind}() got an unexpected keyword {name!r}; " + f"did you mean {suggestion!r}?" + ) + raise TypeError(msg) + # Far from everything chart-shaped: Reflex's CSS convention. + component_kwargs[name] = value + return mark_kwargs, chart_kwargs, chrome, component_kwargs + + +def _data_var_label(data: Any) -> str: + declared = getattr(data, "_original", data) + name = getattr(declared, "_name", None) + return f"data var {name!r}" if isinstance(name, str) and name else "the data var" + + +def _schema_columns(data: Any) -> Optional[frozenset[str]]: + """Column names carried by a ``DataHandle[Schema]``-typed var, if any.""" + var_type = getattr(data, "_var_type", None) + if var_type is None or get_origin(var_type) is not DataHandle: + return None + args = get_args(var_type) + if len(args) != 1 or not is_typeddict(args[0]): + return None + try: + return frozenset(get_type_hints(args[0])) + except Exception: # noqa: BLE001 - schema annotations may not resolve here + return None + + +def _check_schema(plan: ChartPlan, data: Any) -> None: + """R7 compile check: every bound channel exists in the declared schema.""" + schema = _schema_columns(data) + if schema is None: + return + unknown = [name for name in plan.columns if name not in schema] + if unknown: + label = _data_var_label(data) + names = ", ".join(repr(name) for name in unknown) + available = ", ".join(sorted(schema)) + noun = "columns" if len(unknown) > 1 else "column" + msg = f"unknown {noun} {names} for {label}. Available columns: {available}" + raise ValueError(msg) + + +def _mount(plan: ChartPlan, data: Any, component_kwargs: dict[str, Any]) -> Any: + """Attach a compiled plan to its data source and build the component.""" + component_cls = _component() + tooltip = component_kwargs.pop("tooltip", None) + explicit_tailwind = _tailwind_class_tokens(component_kwargs.pop("tailwind_classes", None)) + component_kwargs.setdefault("width", "100%") + component_kwargs.setdefault("height", "420px") + + if isinstance(data, (rx.Var, DataHandle)): + # Live plan tier: digest baked into the JSX, columns resolved from + # the handle at hydrate. The zero-row probe already inventoried the + # chart's DOM classes, so live charts get automatic Tailwind + # discovery (merged with any explicit inventory). + component_kwargs["plan"] = plan.digest + component_kwargs["data"] = data + manifest = _merge_tailwind_class_tokens(plan.tailwind_classes, explicit_tailwind) + if manifest: + component_kwargs["tailwind_class_tokens"] = _tailwind_scan_literal(manifest) + elif isinstance(data, Mapping): + # Static tier: concrete columns bind now; the figure compiles to a + # payload asset and renders kernel-less (works under `reflex export`). + columns = validate_columns(data, source="data=") + figure = plan.bind(columns, source="data=").figure() + component_kwargs["src"] = payload_asset(figure) + manifest = _merge_tailwind_class_tokens(_tailwind_class_manifest(figure), explicit_tailwind) + if manifest: + component_kwargs["tailwind_class_tokens"] = _tailwind_scan_literal(manifest) + elif data is None: + msg = ( + "data= is required: pass a @reflex_xy.data state var for a live " + "chart, or a concrete mapping of columns for a static one" + ) + raise TypeError(msg) + else: + msg = ( + "data= takes a @reflex_xy.data state var, a DataHandle, or a " + f"concrete mapping of columns, got {type(data).__name__}" + ) + raise TypeError(msg) + if tooltip is not None: + return component_cls.create(tooltip, **component_kwargs) + return component_cls.create(**component_kwargs) + + +def _flat_chart(kind: _FlatKind, data: Any, kwargs: dict[str, Any]) -> Any: + mark_kwargs, chart_kwargs, chrome, component_kwargs = _partition_flat(kind, kwargs) + mark = kind.mark_factory(**mark_kwargs) + plan = build_plan(kind.chart_kind, (mark, *chrome), chart_kwargs) + _check_schema(plan, data) + return _mount(plan, data, component_kwargs) + + +def scatter_chart(*, data: Any = None, **kwargs: Any) -> Any: + """A data-bound scatter chart (flat form; see module doc).""" + return _flat_chart(FLAT_KINDS["scatter_chart"], data, kwargs) + + +def line_chart(*, data: Any = None, **kwargs: Any) -> Any: + """A data-bound line chart (flat form; see module doc).""" + return _flat_chart(FLAT_KINDS["line_chart"], data, kwargs) + + +def histogram_chart(*, data: Any = None, **kwargs: Any) -> Any: + """A data-bound histogram (flat form; see module doc).""" + return _flat_chart(FLAT_KINDS["histogram_chart"], data, kwargs) + + +def bar_chart(*, data: Any = None, **kwargs: Any) -> Any: + """A data-bound bar chart (flat form; see module doc).""" + return _flat_chart(FLAT_KINDS["bar_chart"], data, kwargs) + + +def area_chart(*, data: Any = None, **kwargs: Any) -> Any: + """A data-bound area chart (flat form; see module doc).""" + return _flat_chart(FLAT_KINDS["area_chart"], data, kwargs) + + +def step_chart(*, data: Any = None, **kwargs: Any) -> Any: + """A data-bound step chart (flat form; see module doc).""" + return _flat_chart(FLAT_KINDS["step_chart"], data, kwargs) + + +def stem_chart(*, data: Any = None, **kwargs: Any) -> Any: + """A data-bound stem chart (flat form; see module doc).""" + return _flat_chart(FLAT_KINDS["stem_chart"], data, kwargs) + + +def column_chart(*, data: Any = None, **kwargs: Any) -> Any: + """A data-bound column chart (flat form; see module doc).""" + return _flat_chart(FLAT_KINDS["column_chart"], data, kwargs) + + +def errorbar_chart(*, data: Any = None, **kwargs: Any) -> Any: + """A data-bound errorbar chart (flat form; see module doc).""" + return _flat_chart(FLAT_KINDS["errorbar_chart"], data, kwargs) + + +def error_band_chart(*, data: Any = None, **kwargs: Any) -> Any: + """A data-bound error-band chart (flat form; see module doc).""" + return _flat_chart(FLAT_KINDS["error_band_chart"], data, kwargs) + + +def segments_chart(*, data: Any = None, **kwargs: Any) -> Any: + """A data-bound segments chart (flat form; see module doc).""" + return _flat_chart(FLAT_KINDS["segments_chart"], data, kwargs) + + +def chart(*sources: Any, data: Any = None, **kwargs: Any) -> Any: + """Place a chart: composed xy nodes (data-bound), or the component tiers. + + With xy mark/annotation/chrome nodes, this is the composed data-bound + form (Level 2): the nodes are compiled into a plan against ``data=``. + With ``figure=`` or a positional Chart/Figure/token source, it is the + rendering component of the escape hatch and static tiers — + `component.chart` (§5 of reflex-integration.md), unchanged. + """ + if sources and all(isinstance(source, Component) for source in sources): + first = sources[0] + if not (callable(getattr(first, "figure", None)) or isinstance(first, Chart)): + return _composed_chart(sources, data, kwargs) + from .component import chart as component_chart + + if data is not None: + msg = ( + "chart() with data= takes xy mark/chrome nodes " + "(e.g. chart(reflex_xy.scatter('x', 'y'), data=Dash.cloud))" + ) + raise TypeError(msg) + return component_chart(*sources, **kwargs) + + +def _composed_chart(nodes: tuple[Component, ...], data: Any, kwargs: dict[str, Any]) -> Any: + """Level 2: consume plain xy nodes, compile the plan, mount the component.""" + chart_kwargs: dict[str, Any] = {} + component_kwargs: dict[str, Any] = {} + for name, value in kwargs.items(): + if name in EVENT_TRIGGERS or name in COMPONENT_RESERVED: + component_kwargs[name] = value + elif name in CHART_PARAMS: + chart_kwargs[name] = value + elif name.startswith("on_"): + suggestion = _suggest(name, EVENT_TRIGGERS) + if suggestion is not None: + msg = f"chart() got an unexpected event {name!r}; did you mean {suggestion!r}?" + raise TypeError(msg) + component_kwargs[name] = value + else: + suggestion = _suggest(name, CHART_PARAMS | COMPONENT_RESERVED | EVENT_TRIGGERS) + if suggestion is not None: + msg = f"chart() got an unexpected keyword {name!r}; did you mean {suggestion!r}?" + raise TypeError(msg) + component_kwargs[name] = value + plan = build_plan("chart", tuple(nodes), chart_kwargs) + _check_schema(plan, data) + return _mount(plan, data, component_kwargs) diff --git a/spec/design/reflex-integration.md b/spec/design/reflex-integration.md index 882fad99..37551001 100644 --- a/spec/design/reflex-integration.md +++ b/spec/design/reflex-integration.md @@ -473,15 +473,61 @@ drift from what binding will look up), and `.figure()` runs once — the full mark/config validation gate (X1/X2) at compile time, in milliseconds, with no data ingestion (constraint 2). The canonical JSON of the tree (`plan_version: 1`) is content-addressed into a sha256-prefix `digest` and -registered in a process-local `{digest: plan}` map. Binding is the reverse: +registered in a process-local `{digest: plan}` map. Fact X4 ("page bodies +run in every worker") turned out to hold only for processes that run the +frontend compile — **backend-only workers (dev backend subprocesses and +prod workers) import the app module but leave pages unevaluated**, which +would leave their plan maps empty and every plan subscription answering +`err {resync}`. The integration therefore makes X4 true by construction: +`setup(app)`'s lifespan evaluates the app's unevaluated page component +functions once at worker startup (`_ensure_page_plans`), before serving — +factories register their plans as a side effect and the built trees are +discarded (plans and payload assets are content-addressed and idempotent; +a failing page degrades to a warning rather than taking down the data +plane). Binding is the reverse: columns + plan → a **fresh** `Chart` (never reused — X3) → `.figure()`. Column-mismatch errors name both sides (*"plan binds column 'mag'; Dash.cloud produced {x, y}"*). Plans refuse concrete arrays, per-mark `data=`, and `render=` components — data-free structure only. The probe figure also yields `dom_class_strings()`, so **live data-bound charts get automatic Tailwind discovery** (previously live sources needed the manual -inventory). The factories that build plans, and the errors they catch at -`reflex run`, are §5. +inventory). + +**Factories.** `factories.py` provides the flat per-kind forms +(`reflex_xy.scatter_chart(data=…, x="x", y="y", …)`) and the composed +`reflex_xy.chart(*nodes, data=…)` for multi-mark charts, plus curated +re-exports of the xy node constructors (`reflex_xy.scatter` *is* +`xy.scatter`, so a hallucinated constructor dies at import instead of +surviving to hydrate). The kwarg partition between mark options, chrome, +component props, and event handlers is derived from `inspect.signature` at +import — not hand-listed — so it cannot drift from xy's own signatures; +collisions get generated aliases (`mark_`, with `width` becoming +`stroke_width` where the mark hasn't claimed it), pinned by test. Errors +this tier catches at `reflex run`: hallucinated factory names (import), +unknown kwargs with a did-you-mean (partition), bad colormaps/enums/axis +refs (zero-row probe), unknown column names against a typed data var +(schema channel), and the wrong var or a raw string in `data=` (typed +prop, R1). + +**Static tier symmetry.** `data=` given a concrete mapping (not a Var) +binds immediately and routes to the §3.4 payload-asset path — same +validation, same spec-aware bind errors, works under `reflex export`, +never touches the registry. + +**Kind coverage (recorded decision).** Flat factories exist for every mark +kind whose validators compile zero-row — scatter, line, histogram, bar, +area, step, stem, column, errorbar, error_band, segments — each derived +from the mark's signature, and the composed `reflex_xy.chart(*nodes, +data=...)` accepts any mix of those marks plus annotations and chrome. +Aggregating kinds whose validators require at least one finite value (box, +violin, hexbin, contour, heatmap, stairs, ecdf) and the data-taking +composite factories (pie, radar, wind_rose, sankey — eager numeric work at +call time) are **excluded from the plan tier**: the probe refuses them with +an error naming the two supported routes (`@reflex_xy.figure`, or a +concrete xy Chart on the static tier). Extending them would need +value-independent validation or a synthetic-row probe whose failures could +depend on made-up values — rejected as a silent decimation of the compile +guarantee (§28 spirit). **Column entries.** Published columns are registry entries in their own right, keyed by the data token: pure rebuildable caches of Reflex state @@ -851,6 +897,9 @@ python/reflex_xy/ data_vars.py @reflex_xy.data (DataVar: columns in, handle out) plan.py ChartPlan: zero-row probe, canonical digest, process-local plan map, bind (§3.6) + factories.py scatter/line/histogram/bar_chart flat factories + + composed chart(*nodes): signature-derived kwarg + partition, schema checks, plan/data/static mount state_bridge.py token -> state_manager -> builder/data/plan rebuild hooks namespace.py XYNamespace: sub/unsub/msg, payload/msg/err, @@ -858,7 +907,7 @@ python/reflex_xy/ binary attachments app.py setup(app), XYPlugin (post_compile), lifespan component.py chart(figure=...) -> rx.Component (local-JSX - library); typed figure prop; static tier + library); typed figure/data props; static tier payload_asset.py static tier: Chart -> content-addressed XYBF asset in assets/xy/ (§3.4) assets/ XYChart.jsx; links xy's installed render client @@ -876,7 +925,7 @@ examples/reflex/ (repo root) Reflex showcase: figure-var drilldown with whose category toggles re-bin kernel-side, §34) examples/fastapi/ (repo root) the same charts + a live 100M drilldown served from a plain FastAPI app (no committed HTML) -tests/reflex_adapter/ token/registry/var/data-var/plan/bridge/ +tests/reflex_adapter/ token/registry/var/data-var/plan/factory/bridge/ payload-asset units, component compile, framework contract pins (R1/R7/R8), and a real-websocket integration suite (uvicorn + socketio client) diff --git a/tests/reflex_adapter/test_factories.py b/tests/reflex_adapter/test_factories.py new file mode 100644 index 00000000..069c5edc --- /dev/null +++ b/tests/reflex_adapter/test_factories.py @@ -0,0 +1,320 @@ +"""The data-bound chart factories: partition, schema checks, mounting.""" + +from __future__ import annotations + +from typing import TypedDict + +import numpy as np +import pytest +import reflex as rx + +import reflex_xy +import xy +from reflex_xy.factories import EVENT_TRIGGERS, FLAT_KINDS +from reflex_xy.handles import DataHandle + + +class FactorySchema(TypedDict): + x: np.ndarray + y: np.ndarray + mag: np.ndarray + + +class FactoryDash(rx.State): + points: int = 32 + handles: list[DataHandle[FactorySchema]] = [] + + @reflex_xy.data + def cloud(self) -> FactorySchema: + xs = np.linspace(0.0, 1.0, self.points) + return {"x": xs, "y": xs * 0.5, "mag": np.abs(xs)} + + @reflex_xy.data + def untyped(self): + return {"x": [1.0], "y": [2.0]} + + @rx.event + def select(self, selection: dict): + pass + + +@pytest.fixture +def app_cwd(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + import reflex_xy.component as component_mod + + monkeypatch.setattr(component_mod, "_component_cls", None) + return tmp_path + + +def test_partition_table_is_the_public_contract(): + """The generated collision table (options doc §5.6 decision 2): the + component level owns the reserved names; shadowed mark options get + derived aliases — `stroke_width` when free and natural, `mark_` + otherwise. Failing here means the public flat-form surface moved.""" + base = { + "mark_class_name": "class_name", + "mark_opacity": "opacity", + "mark_style": "style", + } + keyed = {**base, "mark_animation": "animation", "mark_key": "key"} + aliases = {kind: FLAT_KINDS[kind].aliases for kind in sorted(FLAT_KINDS)} + assert aliases == { + "area_chart": keyed, + "bar_chart": {**keyed, "mark_width": "width"}, # stroke_width is native + "column_chart": {**keyed, "mark_width": "width"}, # stroke_width is native + "error_band_chart": keyed, + "errorbar_chart": {**keyed, "stroke_width": "width"}, + "histogram_chart": base, + "line_chart": {**keyed, "stroke_width": "width"}, + "scatter_chart": keyed, + "segments_chart": {**base, "stroke_width": "width"}, + "stem_chart": {**base, "stroke_width": "width"}, + "step_chart": {**base, "stroke_width": "width"}, + } + + +def test_event_trigger_set_matches_the_component(app_cwd): + """EVENT_TRIGGERS is derived by hand; pin it against the real component + class so the two can never drift.""" + from reflex_xy.component import _component + + component_triggers = { + name for name in _component().get_event_triggers() if name.startswith("on_") + } + assert component_triggers >= EVENT_TRIGGERS + + +def test_flat_factory_compiles_plan_and_data_props(app_cwd): + comp = reflex_xy.scatter_chart( + data=FactoryDash.cloud, + x="x", + y="y", + color="mag", + colormap="viridis", + height="460px", + on_select_end=FactoryDash.select, + ) + rendered = str(comp) + assert "plan:" in rendered + assert "data:" in rendered + assert "cloud" in rendered + assert "onSelectEnd" in rendered + assert "src" not in rendered + + +def test_flat_factory_validates_mark_config_at_compile(app_cwd): + with pytest.raises(ValueError, match="colormap"): + reflex_xy.scatter_chart(data=FactoryDash.cloud, x="x", y="y", colormap="virids") + + +def test_unknown_kwarg_near_a_known_name_suggests(app_cwd): + with pytest.raises(TypeError, match="did you mean 'colormap'"): + reflex_xy.scatter_chart(data=FactoryDash.cloud, x="x", y="y", colormapp="viridis") + with pytest.raises(TypeError, match="did you mean 'on_select_end'"): + reflex_xy.scatter_chart( + data=FactoryDash.cloud, x="x", y="y", on_selection_end=FactoryDash.select + ) + + +def test_far_off_kwargs_stay_css_passthrough(app_cwd): + """R8's convention, preserved deliberately: unknown non-chart-shaped + kwargs are style props (border_radius here reaches the DOM styles).""" + comp = reflex_xy.scatter_chart(data=FactoryDash.cloud, x="x", y="y", border_radius="12px") + assert "borderRadius" in {str(key) for key in comp.style} + + +def test_typed_schema_rejects_unknown_columns_at_compile(app_cwd): + with pytest.raises( + ValueError, match=r"unknown column 'timestamp'.*Available columns: mag, x, y" + ): + reflex_xy.scatter_chart(data=FactoryDash.cloud, x="timestamp", y="y") + + +def test_schema_check_reaches_foreach_item_vars(app_cwd): + """R7 (foreach half): the element var of list[DataHandle[Schema]] keeps + the schema, so column names are compile-checked inside loops.""" + good = rx.foreach( + FactoryDash.handles, + lambda handle: reflex_xy.scatter_chart(data=handle, x="x", y="y"), + ) + good.render() + + with pytest.raises(ValueError, match="unknown column 'timestamp'"): + rx.foreach( + FactoryDash.handles, + lambda handle: reflex_xy.scatter_chart(data=handle, x="timestamp", y="y"), + ).render() + + +def test_untyped_data_var_skips_the_compile_column_check(app_cwd): + comp = reflex_xy.scatter_chart(data=FactoryDash.untyped, x="anything", y="y") + assert "plan:" in str(comp) + + +def test_wrong_var_and_raw_string_fail_at_compile(app_cwd): + with pytest.raises(TypeError, match="data"): + reflex_xy.scatter_chart(data=FactoryDash.points, x="x", y="y") + with pytest.raises(TypeError, match="data="): + reflex_xy.scatter_chart(data="xyd1|raw|token|string", x="x", y="y") + + +def test_static_tier_routes_concrete_columns_to_payload_asset(app_cwd, _fresh_registry): + comp = reflex_xy.scatter_chart( + data={"x": np.array([1.0, 2.0]), "y": np.array([2.0, 1.0])}, x="x", y="y" + ) + rendered = str(comp) + assert 'src:"/xy/' in rendered + assert "plan:" not in rendered + assert len(_fresh_registry) == 0 # static tier never touches the registry + + +def test_static_tier_bind_errors_name_both_sides(app_cwd): + with pytest.raises(ValueError, match=r"plan binds column 'y'; data= produced \{x\}"): + reflex_xy.scatter_chart(data={"x": [1.0]}, x="x", y="y") + + +def test_static_tier_refuses_kernel_events(app_cwd): + with pytest.raises(ValueError, match=r"on_select_end.*static"): + reflex_xy.scatter_chart( + data={"x": [1.0], "y": [1.0]}, x="x", y="y", on_select_end=FactoryDash.select + ) + + +def test_missing_data_is_an_error(app_cwd): + with pytest.raises(TypeError, match="data= is required"): + reflex_xy.scatter_chart(x="x", y="y") + + +def test_live_charts_discover_tailwind_classes_from_the_probe(app_cwd): + """The zero-row probe figure's class inventory reaches the JSX scan + literal automatically — live plan charts no longer need the manual + tailwind_classes= inventory (the Phase-2 'bonus' in the plan doc).""" + rendered = str( + reflex_xy.scatter_chart( + data=FactoryDash.cloud, + x="x", + y="y", + legend=xy.legend(class_name="max-h-24 overflow-y-auto"), + class_names={"title": "text-base font-semibold"}, + ) + ) + assert "tailwindClassTokens" in rendered + assert "max-h-24 overflow-y-auto" in rendered + assert "text-base font-semibold" in rendered + + +def test_chrome_kwargs_type_dispatch(app_cwd): + comp = reflex_xy.scatter_chart( + data=FactoryDash.cloud, + x="x", + y="y", + x_axis=xy.x_axis(label="sigma"), + legend=True, + ) + assert "plan:" in str(comp) + # a string is the mark's axis-id option, and an undeclared id fails the + # probe exactly like xy itself would at .figure() + with pytest.raises(ValueError, match="axis"): + reflex_xy.scatter_chart(data=FactoryDash.cloud, x="x", y="y", y_axis="y2") + with pytest.raises(TypeError, match="axis node"): + reflex_xy.scatter_chart(data=FactoryDash.cloud, x="x", y="y", x_axis=42) + + +def test_line_stroke_width_alias_and_bar_mark_width(app_cwd): + line = reflex_xy.line_chart(data=FactoryDash.cloud, x="x", y="y", stroke_width=3.0) + assert "plan:" in str(line) + bar = reflex_xy.bar_chart(data=FactoryDash.cloud, x="x", y="y", mark_width=0.5) + assert "plan:" in str(bar) + hist = reflex_xy.histogram_chart(data=FactoryDash.cloud, values="mag", bins=32) + assert "plan:" in str(hist) + + +def test_composed_chart_consumes_xy_nodes(app_cwd): + comp = reflex_xy.chart( + xy.scatter(x="x", y="y", color="mag", colormap="viridis"), + xy.line(x="x", y="y", width=2), + xy.x_axis(label="t"), + data=FactoryDash.cloud, + height="300px", + ) + assert "plan:" in str(comp) + + +def test_composed_chart_checks_schema_across_all_marks(app_cwd): + with pytest.raises(ValueError, match="unknown column 'trend'"): + reflex_xy.chart( + xy.scatter(x="x", y="y"), + xy.line(x="x", y="trend"), + data=FactoryDash.cloud, + ) + + +def test_cond_between_two_composed_charts_validates_both(app_cwd): + """rx.cond builds both branches eagerly (R4), so both plans compile.""" + comp = rx.cond( + FactoryDash.points > 10, + reflex_xy.scatter_chart(data=FactoryDash.cloud, x="x", y="y"), + reflex_xy.line_chart(data=FactoryDash.cloud, x="x", y="y"), + ) + assert "plan:" in str(comp.render()) + + with pytest.raises(ValueError, match="unknown column"): + rx.cond( + FactoryDash.points > 10, + reflex_xy.scatter_chart(data=FactoryDash.cloud, x="x", y="y"), + reflex_xy.line_chart(data=FactoryDash.cloud, x="x", y="bogus"), + ) + + +def test_component_tier_still_reachable_through_chart(app_cwd): + """chart() keeps dispatching the escape hatch and static tiers.""" + static = reflex_xy.chart(xy.line_chart(xy.line([0.0, 1.0], [1.0, 2.0]))) + assert 'src:"/xy/' in str(static) + live = reflex_xy.chart(figure=reflex_xy.FigureHandle("tok")) + assert "figure" in str(live) + with pytest.raises(TypeError, match="data="): + reflex_xy.chart(figure=reflex_xy.FigureHandle("tok"), data=FactoryDash.cloud) + + +def test_curated_reexports_are_the_xy_constructors(): + """`reflex_xy.scatter` *is* `xy.scatter` (zero duplication), and a + hallucinated constructor dies at import against the explicit map.""" + assert reflex_xy.scatter is xy.scatter + assert reflex_xy.x_axis is xy.x_axis + assert reflex_xy.legend is xy.legend + assert reflex_xy.vline is xy.vline + with pytest.raises(AttributeError, match="polar_scatter"): + reflex_xy.polar_scatter # noqa: B018 - the access is the assertion + + +def test_every_flat_kind_compiles_a_plan(app_cwd): + """The signature-derived factories cover every zero-row-safe mark kind.""" + from reflex_xy.factories import FLAT_KINDS + + calls = { + "scatter_chart": dict(x="x", y="y"), + "line_chart": dict(x="x", y="y"), + "histogram_chart": dict(values="x"), + "bar_chart": dict(x="x", y="y"), + "area_chart": dict(x="x", y="y"), + "step_chart": dict(x="x", y="y"), + "stem_chart": dict(x="x", y="y"), + "column_chart": dict(x="x", y="y"), + "errorbar_chart": dict(x="x", y="y", yerr="mag"), + "error_band_chart": dict(x="x", lower="y", upper="mag"), + "segments_chart": dict(x0="x", y0="y", x1="x", y1="mag"), + } + assert set(calls) == set(FLAT_KINDS) + for kind, channels in calls.items(): + comp = getattr(reflex_xy, kind)(data=FactoryDash.cloud, **channels) + assert "plan:" in str(comp), kind + + +def test_needs_data_marks_are_refused_with_guidance(app_cwd): + """The Phase 3 decision: aggregating marks whose validators need at + least one row are excluded from the plan tier, with the escape hatch + and static tier named in the error.""" + with pytest.raises(ValueError, match=r"box.*@reflex_xy\.figure"): + reflex_xy.chart(xy.box("mag"), data=FactoryDash.cloud) + assert not hasattr(reflex_xy, "box_chart") diff --git a/tests/reflex_adapter/test_page_plan_registration.py b/tests/reflex_adapter/test_page_plan_registration.py new file mode 100644 index 00000000..360103ea --- /dev/null +++ b/tests/reflex_adapter/test_page_plan_registration.py @@ -0,0 +1,74 @@ +"""Worker-startup plan registration: X4 made true by construction. + +Backend-only Reflex workers (dev backend subprocesses, prod workers) import +the app module but never run the frontend compile, so page bodies — and the +chart-factory calls inside them that register plans — would never execute +there. `setup(app)`'s lifespan evaluates the app's unevaluated pages once at +startup; these tests pin that seam (reflex-integration.md §3.6). +""" + +from __future__ import annotations + +from types import SimpleNamespace +from typing import TypedDict + +import numpy as np +import pytest +import reflex as rx + +import reflex_xy +from reflex_xy.app import _ensure_page_plans +from reflex_xy.plan import _PLANS + + +class PageSchema(TypedDict): + x: np.ndarray + y: np.ndarray + + +class PagePlanState(rx.State): + @reflex_xy.data + def table(self) -> PageSchema: + return {"x": np.array([1.0]), "y": np.array([2.0])} + + +@pytest.fixture +def app_cwd(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + import reflex_xy.component as component_mod + + monkeypatch.setattr(component_mod, "_component_cls", None) + return tmp_path + + +def test_backend_worker_page_evaluation_registers_plans(app_cwd, _fresh_registry): + def index() -> rx.Component: + return reflex_xy.scatter_chart(data=PagePlanState.table, x="x", y="y") + + app = SimpleNamespace(_unevaluated_pages={"index": SimpleNamespace(component=index)}) + assert not _PLANS # the worker imported the module; nothing evaluated pages + _ensure_page_plans(app) + assert len(_PLANS) == 1 # the factory ran and content-addressed its plan + + +def test_failing_page_warns_and_spares_the_others(app_cwd, _fresh_registry): + def good() -> rx.Component: + return reflex_xy.line_chart(data=PagePlanState.table, x="x", y="y") + + def broken() -> rx.Component: + raise RuntimeError("page body exploded") + + app = SimpleNamespace( + _unevaluated_pages={ + "broken": SimpleNamespace(component=broken), + "good": SimpleNamespace(component=good), + } + ) + with pytest.warns(RuntimeWarning, match="'broken'"): + _ensure_page_plans(app) + assert len(_PLANS) == 1 + + +def test_apps_without_unevaluated_pages_are_a_noop(): + _ensure_page_plans(SimpleNamespace()) # nothing to do, nothing raised + assert not _PLANS From 22a0c62d2a91a88238240e18d4afc4680e304903 Mon Sep 17 00:00:00 2001 From: Farhan Ali Raza Date: Thu, 6 Aug 2026 17:34:52 +0500 Subject: [PATCH 2/6] fix(reflex): strict factory kwargs, fail-closed worker plans, plan-tier bench MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Flat and composed factories reject every unknown top-level kwarg at page evaluation (did-you-mean when close) instead of letting far-off typos silently become CSS; style={...} is the explicit CSS route. - _ensure_page_plans fails closed: a page that cannot evaluate refuses worker startup naming every failing page, rather than warning and serving an incomplete plan map (load-balancer-dependent blank charts). - scripts/bench_reflex_plans.py: reproducible evidence for the tier's new cost centers (plan build 0.26ms, 20x4-chart worker startup 15ms, 100k-point republish 1.0ms, ~1.5KB/plan); recorded in reflex-integration.md §6 with regression contract. - Quickstart multi-mark example binds a column its schema declares. --- python/reflex_xy/__init__.py | 2 +- python/reflex_xy/app.py | 30 ++-- python/reflex_xy/factories.py | 34 ++-- scripts/bench_reflex_plans.py | 169 ++++++++++++++++++ spec/design/reflex-integration.md | 36 +++- tests/reflex_adapter/test_factories.py | 13 +- .../test_page_plan_registration.py | 17 +- 7 files changed, 262 insertions(+), 39 deletions(-) create mode 100644 scripts/bench_reflex_plans.py diff --git a/python/reflex_xy/__init__.py b/python/reflex_xy/__init__.py index 0f012acf..4a690679 100644 --- a/python/reflex_xy/__init__.py +++ b/python/reflex_xy/__init__.py @@ -47,7 +47,7 @@ def index() -> rx.Component: app = rx.App() Multi-mark charts compose xy nodes around the same data var -(``reflex_xy.chart(reflex_xy.scatter("x", "y"), reflex_xy.line("x", "t"), +(``reflex_xy.chart(reflex_xy.scatter("x", "y"), reflex_xy.line("x", "mag"), data=Dash.cloud)``), and charts whose *structure* depends on state keep the escape hatch: an ``@reflex_xy.figure`` method returning an ``xy.Chart``, rendered with ``reflex_xy.chart(figure=Dash.built)`` and probed at compile diff --git a/python/reflex_xy/app.py b/python/reflex_xy/app.py index eae7d30e..e5153d8e 100644 --- a/python/reflex_xy/app.py +++ b/python/reflex_xy/app.py @@ -21,7 +21,6 @@ import asyncio import contextlib -import warnings from typing import Any, Optional from reflex.plugins import Plugin @@ -84,11 +83,18 @@ def _ensure_page_plans(app: Any) -> None: forever. Running the page functions here makes "the plan map is populated in every worker" true by construction; the built component trees are discarded (plans and payload assets are content-addressed and - idempotent). A failing page degrades to a warning: it would have failed - the real compile in the compile process, and one broken page must not - take down the data plane for the others. + idempotent). + + Failure is fail-closed: a page that cannot evaluate here leaves this + worker with an incomplete plan map, and behind a load balancer that is + the worst failure shape there is — charts blank or not depending on + which worker answers, with only a startup warning to explain it. Every + failing page is collected and the worker refuses to start, naming the + pages; the same page code already fails `reflex run`'s real compile, so + a healthy deployment never hits this. """ pages = getattr(app, "_unevaluated_pages", None) or {} + failures: list[str] = [] for route, page in dict(pages).items(): component = getattr(page, "component", None) if not callable(component): @@ -96,13 +102,15 @@ def _ensure_page_plans(app: Any) -> None: try: component() except Exception as exc: # noqa: BLE001 - user page code is an input boundary - warnings.warn( - f"reflex_xy: evaluating page {route!r} for chart-plan " - f"registration failed: {exc!r}. Data-bound charts declared on " - "this page cannot be served by this worker.", - RuntimeWarning, - stacklevel=2, - ) + failures.append(f"{route!r}: {type(exc).__name__}: {exc}") + if failures: + msg = ( + "reflex_xy: evaluating page component functions for chart-plan " + "registration failed on this worker; serving would leave its " + "plan map incomplete (load-balancer-dependent blank charts), " + "so startup is refused. Failing pages: " + "; ".join(sorted(failures)) + ) + raise RuntimeError(msg) def wire(namespace: XYNamespace) -> None: diff --git a/python/reflex_xy/factories.py b/python/reflex_xy/factories.py index 9e1e15e9..a73078cb 100644 --- a/python/reflex_xy/factories.py +++ b/python/reflex_xy/factories.py @@ -38,9 +38,9 @@ Colliding mark options get flat aliases (`stroke_width` for `line`'s `width`, `mark_` otherwise); the resulting table is pinned by `tests/reflex_adapter/test_factories.py` as public contract. Unknown -kwargs near a known name error with a suggestion; anything else falls -through to the component as CSS, preserving Reflex's style convention -(the R8 hazard this partition exists to compensate for). +kwargs always error at page evaluation — with a did-you-mean suggestion +when a known name is close — and never silently become CSS (the R8 hazard +this partition exists to close); explicit ``style={...}`` carries CSS. """ from __future__ import annotations @@ -285,15 +285,19 @@ def _partition_flat( raise TypeError(msg) component_kwargs[name] = value else: + # Strict top-level kwargs: a typo far from every known name must + # not silently become a CSS property on the mount (the promise + # "unknown kwargs fail at page evaluation" holds without a + # distance threshold). CSS stays available, explicitly, through + # the reserved style={...} prop. suggestion = _suggest(name, kind.known) - if suggestion is not None: - msg = ( - f"{kind.chart_kind}() got an unexpected keyword {name!r}; " - f"did you mean {suggestion!r}?" - ) - raise TypeError(msg) - # Far from everything chart-shaped: Reflex's CSS convention. - component_kwargs[name] = value + hint = f"; did you mean {suggestion!r}?" if suggestion is not None else "." + msg = ( + f"{kind.chart_kind}() got an unexpected keyword {name!r}{hint} " + f"CSS belongs in style={{...}}; chart/mark options are listed in " + "the factory docs." + ) + raise TypeError(msg) return mark_kwargs, chart_kwargs, chrome, component_kwargs @@ -479,11 +483,11 @@ def _composed_chart(nodes: tuple[Component, ...], data: Any, kwargs: dict[str, A raise TypeError(msg) component_kwargs[name] = value else: + # Same strict rule as the flat factories: never silently CSS. suggestion = _suggest(name, CHART_PARAMS | COMPONENT_RESERVED | EVENT_TRIGGERS) - if suggestion is not None: - msg = f"chart() got an unexpected keyword {name!r}; did you mean {suggestion!r}?" - raise TypeError(msg) - component_kwargs[name] = value + hint = f"; did you mean {suggestion!r}?" if suggestion is not None else "." + msg = f"chart() got an unexpected keyword {name!r}{hint} CSS belongs in style={{...}}." + raise TypeError(msg) plan = build_plan("chart", tuple(nodes), chart_kwargs) _check_schema(plan, data) return _mount(plan, data, component_kwargs) diff --git a/scripts/bench_reflex_plans.py b/scripts/bench_reflex_plans.py new file mode 100644 index 00000000..6803fe04 --- /dev/null +++ b/scripts/bench_reflex_plans.py @@ -0,0 +1,169 @@ +"""Benchmark the data-bound chart tier's page-time and serve-time costs. + +The plan tier moves work to two places the rest of the benchmark program +does not cover: page evaluation (every chart factory call compiles + probes +a plan) and backend-worker startup (`_ensure_page_plans` re-evaluates every +page). Both must stay compile-scale (milliseconds), and a column republish +must stay dominated by the figure build it fans out, or the tier's promise +("state deltas independent of data size, republish = one screen-bounded +reship") quietly erodes. This harness measures all of it reproducibly; +recorded results live in spec/design/reflex-integration.md §6. + +Run (needs the reflex extra): + + uv run python scripts/bench_reflex_plans.py [--json] +""" + +from __future__ import annotations + +import argparse +import json +import statistics +import sys +import time +import tracemalloc +from collections.abc import Callable +from types import SimpleNamespace +from typing import Any + +REPEATS = 30 +REPUBLISH_POINTS = 100_000 +STARTUP_PAGES = 20 +CHARTS_PER_PAGE = 4 + + +def _median_ms(fn: Callable[[], Any], repeats: int = REPEATS) -> float: + times = [] + for _ in range(repeats): + start = time.perf_counter() + fn() + times.append((time.perf_counter() - start) * 1e3) + return statistics.median(times) + + +def bench_plan_build() -> float: + """One flat factory call at page evaluation (compile + probe + digest).""" + import xy + from reflex_xy.plan import build_plan, reset_plans_for_tests + + def build() -> None: + reset_plans_for_tests() # avoid the map turning builds into lookups + build_plan( + "scatter_chart", + (xy.scatter("x", "y", color="mag", colormap="viridis"), xy.x_axis(label="sigma")), + {"title": "cloud"}, + ) + + return _median_ms(build) + + +def bench_worker_startup() -> float: + """_ensure_page_plans over a synthetic app: pages x charts per worker boot.""" + import reflex_xy + from reflex_xy.app import _ensure_page_plans + from reflex_xy.handles import DataHandle + from reflex_xy.plan import reset_plans_for_tests + + def page(i: int) -> Callable[[], Any]: + def body() -> Any: + # live tier (DataHandle): the page cost is plan compile + probe + + # component mount, with no payload-asset writes involved + return [ + reflex_xy.chart( + reflex_xy.scatter("x", "y", opacity=0.4 + 0.001 * (i * CHARTS_PER_PAGE + j)), + data=DataHandle(""), + ) + for j in range(CHARTS_PER_PAGE) + ] + + return body + + app = SimpleNamespace( + _unevaluated_pages={ + f"page{i}": SimpleNamespace(component=page(i)) for i in range(STARTUP_PAGES) + } + ) + + def boot() -> None: + reset_plans_for_tests() + _ensure_page_plans(app) + + return _median_ms(boot, repeats=5) + + +def bench_republish() -> float: + """publish_columns -> dependent bind + figure build + publish, mounted.""" + import numpy as np + + import xy + from reflex_xy.plan import build_plan + from reflex_xy.registry import FigureRegistry + from reflex_xy.tokens import build_plan_token + + registry = FigureRegistry() + plan = build_plan("scatter_chart", (xy.scatter("x", "y"),), {}) + data_token = "xyd1|bench-client-token|app.app.State|cloud" + composite = build_plan_token(plan.digest, data_token) + registry.subscribe(composite, "bench-sid", rebuildable=True) + registry.bind_plan(data_token, plan.digest) + rng = np.random.default_rng(7) + xs = rng.normal(size=REPUBLISH_POINTS) + columns = {"x": xs, "y": xs * 0.5} + + def republish() -> None: + registry.publish_columns(data_token, columns) + + republish() # prime the mount + return _median_ms(republish, repeats=10) + + +def bench_plan_memory() -> float: + """Resident bytes per registered plan (the worker-lifetime map entry).""" + import xy + from reflex_xy.plan import build_plan, reset_plans_for_tests + + reset_plans_for_tests() + count = 200 + tracemalloc.start() + before = tracemalloc.take_snapshot() + for i in range(count): + build_plan("scatter_chart", (xy.scatter("x", "y", opacity=i / count),), {}) + after = tracemalloc.take_snapshot() + tracemalloc.stop() + total = sum(stat.size_diff for stat in after.compare_to(before, "filename")) + reset_plans_for_tests() + return total / count + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--json", action="store_true", help="machine-readable output") + args = parser.parse_args() + + results = { + "plan_build_ms": round(bench_plan_build(), 3), + "worker_startup_ms": round(bench_worker_startup(), 1), + "worker_startup_pages": STARTUP_PAGES, + "worker_startup_charts": STARTUP_PAGES * CHARTS_PER_PAGE, + "republish_ms": round(bench_republish(), 1), + "republish_points": REPUBLISH_POINTS, + "plan_memory_bytes": round(bench_plan_memory()), + } + if args.json: + print(json.dumps(results, indent=2)) + return 0 + print(f"plan build (compile+probe+digest) {results['plan_build_ms']:8.3f} ms median") + print( + f"worker startup page evaluation {results['worker_startup_ms']:8.1f} ms " + f"({STARTUP_PAGES} pages x {CHARTS_PER_PAGE} charts)" + ) + print( + f"column republish -> new payload {results['republish_ms']:8.1f} ms " + f"({REPUBLISH_POINTS:,} points, mounted)" + ) + print(f"plan map entry {results['plan_memory_bytes']:8.0f} bytes/plan") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/spec/design/reflex-integration.md b/spec/design/reflex-integration.md index 37551001..246866b8 100644 --- a/spec/design/reflex-integration.md +++ b/spec/design/reflex-integration.md @@ -482,9 +482,14 @@ would leave their plan maps empty and every plan subscription answering `setup(app)`'s lifespan evaluates the app's unevaluated page component functions once at worker startup (`_ensure_page_plans`), before serving — factories register their plans as a side effect and the built trees are -discarded (plans and payload assets are content-addressed and idempotent; -a failing page degrades to a warning rather than taking down the data -plane). Binding is the reverse: +discarded (plans and payload assets are content-addressed and idempotent). +Failure is **fail-closed**: a page that cannot evaluate would leave this +worker's plan map incomplete — behind a load balancer that means charts +blank or not depending on which worker answers — so `_ensure_page_plans` +collects every failing page and refuses worker startup with an error +naming them, instead of serving inconsistently. (The same page code fails +`reflex run`'s real compile first, so a healthy deployment never hits +this.) Binding is the reverse: columns + plan → a **fresh** `Chart` (never reused — X3) → `.figure()`. Column-mismatch errors name both sides (*"plan binds column 'mag'; Dash.cloud produced {x, y}"*). Plans refuse concrete arrays, per-mark @@ -502,9 +507,14 @@ surviving to hydrate). The kwarg partition between mark options, chrome, component props, and event handlers is derived from `inspect.signature` at import — not hand-listed — so it cannot drift from xy's own signatures; collisions get generated aliases (`mark_`, with `width` becoming -`stroke_width` where the mark hasn't claimed it), pinned by test. Errors +`stroke_width` where the mark hasn't claimed it), pinned by test. Top-level +kwargs are **strict**: an unknown name always raises `TypeError` at page +evaluation — with a did-you-mean when a known name is close — and never +silently becomes a CSS property (the R8 hazard, closed rather than +compensated for by a distance threshold); CSS goes through the explicit +`style={...}` prop, which reaches the DOM unchanged. Errors this tier catches at `reflex run`: hallucinated factory names (import), -unknown kwargs with a did-you-mean (partition), bad colormaps/enums/axis +unknown kwargs (partition), bad colormaps/enums/axis refs (zero-row probe), unknown column names against a typed data var (schema channel), and the wrong var or a raw string in `data=` (typed prop, R1). @@ -883,6 +893,22 @@ changes — builders are user code and should be O(state); heavy shared data prep belongs outside the builder (module cache / backend var), which the demo app models. +**Plan-tier costs (measured).** The data-bound tier moves work to page +evaluation and worker startup; `scripts/bench_reflex_plans.py` measures it +reproducibly (`uv run python scripts/bench_reflex_plans.py [--json]`). +Recorded 2026-08-06 (Linux, Python 3.12, native core, dev machine): + +| metric | recorded | scale contract | +|---|---|---| +| plan build (compile + probe + digest) | 0.26 ms median | per chart factory call, milliseconds — page evaluation stays compile-scale | +| worker startup page evaluation | 15 ms (20 pages × 4 charts) | `_ensure_page_plans` re-runs pages once per worker boot; linear in charts | +| column republish → new mounted payload | 1.0 ms (100k points) | dominated by the dependent figure build, not registry bookkeeping | +| plan map entry | ~1.5 KB/plan | process-lifetime map, bounded by page code | + +Re-record when the probe or serialization changes materially; a plan build +drifting toward tens of milliseconds or a republish cost growing faster +than its figure build is a regression against this table. + ## 7. What shipped where (prototype map) ``` diff --git a/tests/reflex_adapter/test_factories.py b/tests/reflex_adapter/test_factories.py index 069c5edc..d8b44682 100644 --- a/tests/reflex_adapter/test_factories.py +++ b/tests/reflex_adapter/test_factories.py @@ -117,10 +117,15 @@ def test_unknown_kwarg_near_a_known_name_suggests(app_cwd): ) -def test_far_off_kwargs_stay_css_passthrough(app_cwd): - """R8's convention, preserved deliberately: unknown non-chart-shaped - kwargs are style props (border_radius here reaches the DOM styles).""" - comp = reflex_xy.scatter_chart(data=FactoryDash.cloud, x="x", y="y", border_radius="12px") +def test_far_off_kwargs_error_and_point_at_style(app_cwd): + """Strict top-level kwargs: a typo far from every known name errors at + page evaluation instead of silently becoming CSS (the R8 hazard); + explicit style={...} is the CSS route and still reaches the DOM.""" + with pytest.raises(TypeError, match=r"borderr_radus.*style=\{"): + reflex_xy.scatter_chart(data=FactoryDash.cloud, x="x", y="y", borderr_radus="12px") + comp = reflex_xy.scatter_chart( + data=FactoryDash.cloud, x="x", y="y", style={"border_radius": "12px"} + ) assert "borderRadius" in {str(key) for key in comp.style} diff --git a/tests/reflex_adapter/test_page_plan_registration.py b/tests/reflex_adapter/test_page_plan_registration.py index 360103ea..b3b812c6 100644 --- a/tests/reflex_adapter/test_page_plan_registration.py +++ b/tests/reflex_adapter/test_page_plan_registration.py @@ -51,22 +51,33 @@ def index() -> rx.Component: assert len(_PLANS) == 1 # the factory ran and content-addressed its plan -def test_failing_page_warns_and_spares_the_others(app_cwd, _fresh_registry): +def test_failing_page_refuses_worker_startup(app_cwd, _fresh_registry): + """Fail closed: a worker whose plan map would be incomplete must not + serve (blank charts depending on which worker the balancer picks). The + error names every failing page; healthy pages still registered, so a + fixed deployment starts clean.""" + def good() -> rx.Component: return reflex_xy.line_chart(data=PagePlanState.table, x="x", y="y") def broken() -> rx.Component: raise RuntimeError("page body exploded") + def also_broken() -> rx.Component: + raise ValueError("second page exploded") + app = SimpleNamespace( _unevaluated_pages={ "broken": SimpleNamespace(component=broken), "good": SimpleNamespace(component=good), + "worse": SimpleNamespace(component=also_broken), } ) - with pytest.warns(RuntimeWarning, match="'broken'"): + with pytest.raises(RuntimeError, match=r"'broken'.*'worse'") as excinfo: _ensure_page_plans(app) - assert len(_PLANS) == 1 + assert "page body exploded" in str(excinfo.value) + assert "second page exploded" in str(excinfo.value) + assert len(_PLANS) == 1 # the good page registered before the refusal def test_apps_without_unevaluated_pages_are_a_noop(): From b9ec0f74d51ad6c71ab1d3d3d5b094711be5a7cb Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Thu, 6 Aug 2026 14:39:14 -0700 Subject: [PATCH 3/6] fix(docs): bind each demo fence to its own namespace on re-render The shared Markdown renderer runs every exec fence of a page into one synthetic module and skips re-executing a fence it has already run, so a demo's preview function resolves its data from that module at call time. `_ensure_page_plans` evaluates page bodies a second time in the same process, and on that pass every preview read the end-of-page namespace -- whatever the last fence bound `months`/`x`/`y` to. Pages that reuse names rebuilt from another demo's data: styling/customize and styling/examples failed outright on mismatched lengths and refused worker startup, and where lengths agreed the second pass silently minted plan digests the compiled frontend never references. Snapshot the namespace at the end of each fence's first execution and restore it in place -- the module dict object is what the fence's functions close over -- before that fence renders again, so every render sees the namespace the first one did. Cover it with a repeated-render test over every page whose fences rebind a name, asserting identical content-addressed payload sources, and record the re-evaluability contract that page-plan registration puts on app code. --- docs/app/tests/test_docs_site.py | 58 +++++++++++++++++++++++++++++ docs/app/xy_docs/markdown.py | 62 ++++++++++++++++++++++++++++++- spec/design/reflex-integration.md | 13 +++++-- 3 files changed, 129 insertions(+), 4 deletions(-) diff --git a/docs/app/tests/test_docs_site.py b/docs/app/tests/test_docs_site.py index 94544dc4..dc06c2f4 100644 --- a/docs/app/tests/test_docs_site.py +++ b/docs/app/tests/test_docs_site.py @@ -1155,6 +1155,64 @@ def render() -> str: assert second_payloads == first_payloads +def _names_rebound_across_exec_fences(content: str) -> set[str]: + """Top-level names that more than one exec fence of a page assigns.""" + seen: set[str] = set() + rebound: set[str] = set() + for fence, body in re.findall(r"~~~python([^\n]*)\n(.*?)\n~~~", content, re.DOTALL): + if "exec" not in fence: + continue + assigned = { + target.id + for node in ast.parse(body).body + if isinstance(node, ast.Assign) + for target in node.targets + if isinstance(target, ast.Name) + } + rebound |= assigned & seen + seen |= assigned + return rebound + + +def test_repeated_page_renders_rebuild_every_demo_from_its_own_data( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """Keep cached fences bound to their own globals, not the page's last ones. + + Pages are evaluated more than once per process — the frontend compile, then + the `reflex_xy` worker-startup pass over unevaluated pages — and demos that + reuse names such as `months` must not pick up a later demo's arrays on the + second pass. Content-addressed payload names make that visible: identical + demo data renders identical `.xyf` sources. + """ + monkeypatch.chdir(tmp_path) + payload_pattern = re.compile(r'src:"(?:/docs/xy)?/xy/([^"]+\.xyf)"') + pages = [ + page + for page in discover_docs(DOCS_CONFIG) + if _names_rebound_across_exec_fences(page.content) + ] + + assert pages + for page in pages: + route = page.relative_path.as_posix() + virtual_filepath = f"tests/docdemo/repeated-render/{route}" + renders = [ + str( + XyDocsMarkdownTransformer( + virtual_filepath=virtual_filepath, + filename=page.source_path.as_posix(), + ).transform(parse_document(page.content)) + ) + for _ in range(2) + ] + + first, second = (payload_pattern.findall(rendered) for rendered in renders) + assert first, route + assert second == first, route + + def test_exact_readout_demo_is_backed_by_live_reflex_state() -> None: """Keep the readout driven by chart events instead of compiled sample values.""" source_path = DOCS_ROOT / "core-concepts" / "interactions.md" diff --git a/docs/app/xy_docs/markdown.py b/docs/app/xy_docs/markdown.py index 13596a89..27779750 100644 --- a/docs/app/xy_docs/markdown.py +++ b/docs/app/xy_docs/markdown.py @@ -7,7 +7,13 @@ import reflex as rx from reflex_docgen.markdown import CodeBlock, HeadingBlock, parse_document -from reflex_site_shared.docs.markdown import ReflexDocTransformer, _spans_to_plaintext +from reflex_site_shared.docs.markdown import ( + ReflexDocTransformer, + _exec_code, + _file_modules, + _last_defined_name, + _spans_to_plaintext, +) from reflex_site_shared.docs.models import DocsPage from reflex_site_shared.views.hosting_banner import HostingBannerState @@ -28,6 +34,10 @@ _DEMO_DATA_DIVIDER = "# --- chart ---" _DEMO_DATA_TAB_LINE_THRESHOLD = 10 +# Namespace of the page module as of the end of each executed fence, keyed by +# (virtual filepath, fence source). See `_exec_fence`. +_FENCE_NAMESPACES: dict[tuple[str, str], dict] = {} + def _split_demo_data(content: str) -> tuple[str | None, str]: """Split a demo fence into (data, code) around ``_DEMO_DATA_DIVIDER``. @@ -109,10 +119,60 @@ def code_block(self, block: CodeBlock) -> rx.Component: """Use the accessible XY code block for every visible source fence.""" flags = set(block.flags) language = block.language or "plain" + if language == "python" and "exec" in flags and not flags & {"demo", "demo-only"}: + # Same contract as the shared renderer's bare-``exec`` branch, but + # through the snapshot-aware seam below. + self._exec_fence(block.content) + return rx.fragment() if language == "python" and flags.intersection({"demo", "demo-only", "exec", "eval"}): return super().code_block(block) return code_block(block.content, language) + def _exec_fence(self, content: str) -> None: + """Execute one fence, or restore the namespace it left behind. + + Every exec fence in a page shares one synthetic module, and the shared + renderer skips re-executing a fence it has already run (State classes + must not be redefined). A page is evaluated more than once per process + — the frontend compile, then `reflex_xy`'s worker-startup pass over the + unevaluated pages (reflex-integration.md §"Plans") — and on those later + renders the module namespace holds *end-of-page* values, so a fence's + preview function reads whatever a later fence last bound its names to. + Demos that reuse names (`months`, `x`, `y`) then build from another + demo's data: silently different charts, mismatched plan digests, or a + hard failure when the shadowing arrays disagree in length. + + So snapshot the namespace at the end of each fence's first execution + and restore it in place (the module dict object is what the fence's + functions close over) before the fence renders again. Every render + sees exactly the namespace the first one did. + """ + key = (self.virtual_filepath, content) + namespace = _FENCE_NAMESPACES.get(key) + if namespace is None: + _exec_code(content, self.env, self.virtual_filepath) + _FENCE_NAMESPACES[key] = dict(self.env) + return + module = _file_modules.get(self.virtual_filepath) + if module is not None: + module.__dict__.clear() + module.__dict__.update(namespace) + self.env.clear() + self.env.update(namespace) + + def _exec_and_get_last_callable(self, content: str): + """Call the fence's last-defined callable against its own namespace.""" + self._exec_fence(content) + last_name = _last_defined_name(content) + if last_name is None: + msg = "Exec block defines no function or class" + raise RuntimeError(msg) + last = self.env[last_name] + if not callable(last): + msg = f"Last defined name {last_name!r} is not callable" + raise TypeError(msg) + return last() + def _render_demo(self, content: str, flags: set[str]) -> rx.Component: """Render public chart demos with consistent Preview/Code/Data tabs.""" component_id = next( diff --git a/spec/design/reflex-integration.md b/spec/design/reflex-integration.md index 246866b8..e7ea92b7 100644 --- a/spec/design/reflex-integration.md +++ b/spec/design/reflex-integration.md @@ -487,9 +487,16 @@ Failure is **fail-closed**: a page that cannot evaluate would leave this worker's plan map incomplete — behind a load balancer that means charts blank or not depending on which worker answers — so `_ensure_page_plans` collects every failing page and refuses worker startup with an error -naming them, instead of serving inconsistently. (The same page code fails -`reflex run`'s real compile first, so a healthy deployment never hits -this.) Binding is the reverse: +naming them, instead of serving inconsistently. The contract this puts on +app code is **re-evaluability**: a page body runs at least twice per +process (the compile, then this pass) and must build the same charts each +time. Code that mutates module state a later page body reads — the docs +site's Markdown demo runner cached exec fences in one page-wide module +namespace, so a second render rebuilt each demo from the *last* fence's +data — either changes plan digests (leaving the compiled frontend's token +unregistered) or fails outright here; a page that only fails on the second +evaluation still passes `reflex run`'s compile, so the fail-closed error is +the check that catches it. Binding is the reverse: columns + plan → a **fresh** `Chart` (never reused — X3) → `.figure()`. Column-mismatch errors name both sides (*"plan binds column 'mag'; Dash.cloud produced {x, y}"*). Plans refuse concrete arrays, per-mark From 3b6abdb76c52f00da9bd69eb06bc30b9d34136f0 Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Thu, 6 Aug 2026 22:03:14 +0000 Subject: [PATCH 4/6] fix(reflex): restore the resync retry budget on reconnect; export stem_chart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The errResyncs cap guards against same-connection err{resync} loops, but it also dead-ended a chart whose backend recovered after five failed attempts with nothing short of a remount. A fresh connection now resets the budget alongside the resubscribe it already triggers. stem_chart was implemented, registered, and importable but missing from factories.__all__ — the one flat factory absent from the public list. --- python/reflex_xy/assets/XYChart.jsx | 11 +++++++++-- python/reflex_xy/factories.py | 1 + 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/python/reflex_xy/assets/XYChart.jsx b/python/reflex_xy/assets/XYChart.jsx index c5d24697..51f64645 100644 --- a/python/reflex_xy/assets/XYChart.jsx +++ b/python/reflex_xy/assets/XYChart.jsx @@ -932,7 +932,14 @@ export function XYChart(props) { // Resubscribe on every (re)connect: after the app plane reconnects the // shared manager, rooms are gone and — on another backend node — the // figure itself may need a state-driven rebuild. `sub` triggers both. - socket.on("connect", subscribe); + // A fresh connection also restores the bounded resync-retry budget: the + // cap exists to stop same-connection err loops, not to dead-end a chart + // whose backend recovered after the budget ran out. + const onConnect = () => { + errResyncs = 0; + subscribe(); + }; + socket.on("connect", onConnect); subCounts.set(liveToken, (subCounts.get(liveToken) || 0) + 1); if (socket.connected) subscribe(); @@ -963,7 +970,7 @@ export function XYChart(props) { socket.off("msg", onMsg); socket.off("err", onErr); socket.off("disconnect", onDisconnect); - socket.off("connect", subscribe); + socket.off("connect", onConnect); const remaining = (subCounts.get(liveToken) || 1) - 1; if (remaining <= 0) { subCounts.delete(liveToken); diff --git a/python/reflex_xy/factories.py b/python/reflex_xy/factories.py index a73078cb..0225b6d9 100644 --- a/python/reflex_xy/factories.py +++ b/python/reflex_xy/factories.py @@ -79,6 +79,7 @@ "line_chart", "scatter_chart", "segments_chart", + "stem_chart", "step_chart", ] From af9f99c58548f275299e4dc58792505021b770d9 Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Thu, 6 Aug 2026 23:35:12 +0000 Subject: [PATCH 5/6] fix(reflex): make the worker-startup refusal actually refuse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_ensure_page_plans` was fail-closed in name only. Reflex starts a coroutine lifespan task with `asyncio.create_task(task())` and then yields, so a raise inside an `async def` lifespan body landed in a background task on an already-serving worker — the load-balancer-dependent blank charts the check exists to prevent. The registered task is now a plain function that runs the page pass synchronously and returns the sweep coroutine, so the raise happens in the `task()` call Reflex makes inline, before create_task and before the lifespan yields. Alongside it, four narrower corrections: - The signature-derived kwarg partition excluded only `data`, so xy's private adapter knobs (`_artist_alpha`, `_marker_path`, …) were accepted as flat kwargs and offered as did-you-mean candidates. Underscore-prefixed params are now filtered out of both. - Every public `reflex_xy` export is restated under `TYPE_CHECKING` — the twelve chart factories and the curated xy node re-exports, which typed as missing/`Any` behind `__getattr__`. A new test pins `__all__` against the static declarations in both directions. - Demo-fence namespace snapshots key on (page, source, occurrence) instead of (page, source). A page repeating an identical fence would otherwise restore the first occurrence's namespace, discarding what the fences between them defined — not the shared renderer's accumulation semantics. One occurrence counter is threaded through the body and FAQ transformers. - The republish benchmark sweeps data size (10k → 5M, straddling the 200k density threshold) and reports ms per million points. "State deltas independent of data size" is a scaling claim; one datum at one N is consistent with any growth curve. Table in §6 replaced with the sweep. Also: the repeated-render docs guard now detects annotated, augmented, unpacked, and def/class rebinding, not just plain assignment (12 → 13 pages covered), and the fourfold `app_cwd` fixture is hoisted into `tests/reflex_adapter/conftest.py`. --- docs/app/tests/test_docs_site.py | 127 ++++++++++++++++-- docs/app/xy_docs/markdown.py | 39 +++++- python/reflex_xy/__init__.py | 63 ++++++++- python/reflex_xy/app.py | 26 ++-- python/reflex_xy/factories.py | 13 +- scripts/bench_reflex_plans.py | 73 ++++++++-- spec/design/reflex-integration.md | 48 ++++++- tests/reflex_adapter/conftest.py | 17 +++ tests/reflex_adapter/test_component.py | 12 -- tests/reflex_adapter/test_factories.py | 25 ++-- .../test_page_plan_registration.py | 63 +++++++-- tests/reflex_adapter/test_payload_asset.py | 9 -- tests/reflex_adapter/test_public_surface.py | 72 ++++++++++ 13 files changed, 507 insertions(+), 80 deletions(-) create mode 100644 tests/reflex_adapter/test_public_surface.py diff --git a/docs/app/tests/test_docs_site.py b/docs/app/tests/test_docs_site.py index dc06c2f4..30792880 100644 --- a/docs/app/tests/test_docs_site.py +++ b/docs/app/tests/test_docs_site.py @@ -1155,25 +1155,132 @@ def render() -> str: assert second_payloads == first_payloads +def test_repeated_identical_fence_keeps_the_page_namespace_accumulating( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """A duplicated fence must not roll the page back to its first occurrence. + + Snapshots are keyed by fence source *and* occurrence: two identical + fences are two positions in the page's fence sequence, and the second one + restoring the first one's namespace would discard everything the fences + between them defined — the shared renderer skips re-execution and lets + the namespace keep accumulating, and this seam must not change that. + """ + duplicated = "shared = 1\n" + content = "\n".join( + ( + "~~~python exec", + duplicated.rstrip("\n"), + "~~~", + "", + "~~~python exec", + "between = 2", + "~~~", + "", + "~~~python exec", + duplicated.rstrip("\n"), + "~~~", + "", + "~~~python exec", + "def result():", + " return (shared, between)", + "~~~", + ) + ) + virtual_filepath = "tests/docdemo/repeated-fence/page.md" + + def render() -> dict: + transformer = XyDocsMarkdownTransformer( + virtual_filepath=virtual_filepath, + filename=virtual_filepath, + ) + transformer.transform(parse_document(content)) + return transformer.env + + monkeypatch.chdir(tmp_path) + first, second = render(), render() + # `between` survives the duplicate fence on the first render and on every + # later one; a KeyError here is the regression. + assert first["result"]() == (1, 2) + assert second["result"]() == (1, 2) + + +def _assigned_names(target: ast.expr) -> set[str]: + """Every top-level name an assignment target binds, unpacking included.""" + if isinstance(target, ast.Name): + return {target.id} + if isinstance(target, (ast.Tuple, ast.List)): + return {name for element in target.elts for name in _assigned_names(element)} + if isinstance(target, ast.Starred): + return _assigned_names(target.value) + return set() # attribute/subscript targets rebind no module-level name + + +def _bound_names(body: str) -> set[str]: + """Top-level names one fence binds, in every form a demo can use. + + Plain assignment is not the only way to shadow a name a sibling fence + also owns: `x: list = ...`, `x += ...`, `x, y = ...`, and `def`/`class` + all do it. Matching only `ast.Assign` would skip those pages, leaving the + regression this guards unexercised on exactly the demos most likely to + hit it. + """ + bound: set[str] = set() + for node in ast.parse(body).body: + if isinstance(node, ast.Assign): + for target in node.targets: + bound |= _assigned_names(target) + elif isinstance(node, (ast.AnnAssign, ast.AugAssign)): + bound |= _assigned_names(node.target) + elif isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + bound.add(node.name) + return bound + + def _names_rebound_across_exec_fences(content: str) -> set[str]: - """Top-level names that more than one exec fence of a page assigns.""" + """Top-level names that more than one exec fence of a page binds.""" seen: set[str] = set() rebound: set[str] = set() for fence, body in re.findall(r"~~~python([^\n]*)\n(.*?)\n~~~", content, re.DOTALL): if "exec" not in fence: continue - assigned = { - target.id - for node in ast.parse(body).body - if isinstance(node, ast.Assign) - for target in node.targets - if isinstance(target, ast.Name) - } - rebound |= assigned & seen - seen |= assigned + bound = _bound_names(body) + rebound |= bound & seen + seen |= bound return rebound +def test_rebound_name_detection_covers_every_binding_form() -> None: + """Pin the detector itself: the page filter above is only as complete as + the binding forms it recognizes.""" + fence = "~~~python exec\n{body}\n~~~" + page = "\n".join( + fence.format(body=body) + for body in ( + "plain = 1\nannotated: int = 1\naugmented = 0\nunpacked = 0\nspread = 0\n" + "def fn():\n pass\n\n\nclass Cls:\n pass", + "plain = 2", + "annotated: int = 2", + "augmented += 2", + "unpacked, other = (2, 3)", + "*spread, tail = [2, 3]", + "def fn():\n return 2", + "class Cls:\n pass", + "obj.attr = 2\nmapping['key'] = 2", # rebinds no module-level name + ) + ) + assert _names_rebound_across_exec_fences(page) == { + "plain", + "annotated", + "augmented", + "unpacked", + "spread", + "fn", + "Cls", + } + + def test_repeated_page_renders_rebuild_every_demo_from_its_own_data( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, diff --git a/docs/app/xy_docs/markdown.py b/docs/app/xy_docs/markdown.py index 27779750..90a747a2 100644 --- a/docs/app/xy_docs/markdown.py +++ b/docs/app/xy_docs/markdown.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +from collections import Counter from dataclasses import replace import reflex as rx @@ -35,8 +36,11 @@ _DEMO_DATA_TAB_LINE_THRESHOLD = 10 # Namespace of the page module as of the end of each executed fence, keyed by -# (virtual filepath, fence source). See `_exec_fence`. -_FENCE_NAMESPACES: dict[tuple[str, str], dict] = {} +# (virtual filepath, fence source, occurrence of that source in the page). The +# occurrence disambiguates a page that repeats an identical fence: those are +# distinct positions in the page's fence sequence and saw different namespaces. +# See `_exec_fence`. +_FENCE_NAMESPACES: dict[tuple[str, str, int], dict] = {} def _split_demo_data(content: str) -> tuple[str | None, str]: @@ -111,6 +115,21 @@ def _heading_link(text: str, level: int) -> rx.Component: class XyDocsMarkdownTransformer(ReflexDocTransformer): """Render XY docs while keeping heading links independent of router state.""" + def __init__( + self, + virtual_filepath: str = "", + filename: str = "", + fence_occurrences: Counter[str] | None = None, + ) -> None: + super().__init__(virtual_filepath=virtual_filepath, filename=filename) + # Counts exec fences by source as this render walks the page, so the + # nth identical fence keys its own snapshot. `render_xy_markdown_page` + # renders a page's body and its FAQ through two transformers; passing + # one counter through keeps a single fence sequence across both. + self.fence_occurrences: Counter[str] = ( + Counter() if fence_occurrences is None else fence_occurrences + ) + def heading(self, block: HeadingBlock) -> rx.Component: """Render one route-local Markdown heading.""" return _heading_link(_spans_to_plaintext(block.children), block.level) @@ -146,8 +165,18 @@ def _exec_fence(self, content: str) -> None: and restore it in place (the module dict object is what the fence's functions close over) before the fence renders again. Every render sees exactly the namespace the first one did. + + A fence is identified by its source *and* its occurrence index within + the page, never by source alone: a page that repeats an identical + fence has two positions in its fence sequence, and keying on source + alone would make the second one restore the first one's snapshot — + discarding whatever the fences in between defined, which is not what + the shared renderer does (it skips re-execution and lets the page's + namespace keep accumulating). """ - key = (self.virtual_filepath, content) + occurrence = self.fence_occurrences[content] + self.fence_occurrences[content] += 1 + key = (self.virtual_filepath, content, occurrence) namespace = _FENCE_NAMESPACES.get(key) if namespace is None: _exec_code(content, self.env, self.virtual_filepath) @@ -201,11 +230,15 @@ def _render_demo(self, content: str, flags: set[str]) -> rx.Component: def render_xy_markdown_page(page: DocsPage) -> rx.Component: """Render one discovered XY documentation page.""" source_path = page.source_path.resolve() + # One fence sequence per page render, even though the body and the FAQ are + # transformed separately around the generated API section. + fence_occurrences: Counter[str] = Counter() def _render(markdown_text: str) -> rx.Component: transformer = XyDocsMarkdownTransformer( virtual_filepath=str(source_path), filename=str(source_path), + fence_occurrences=fence_occurrences, ) return transformer.transform(parse_document(markdown_text)) diff --git a/python/reflex_xy/__init__.py b/python/reflex_xy/__init__.py index 4a690679..42426962 100644 --- a/python/reflex_xy/__init__.py +++ b/python/reflex_xy/__init__.py @@ -381,6 +381,54 @@ def release(token: "str | FigureHandle") -> None: if TYPE_CHECKING: + # The curated xy node re-exports (`_XY_REEXPORTS`) resolve at runtime + # through `__getattr__`; restate them here so a type checker sees the real + # constructor signatures instead of `Any` (or nothing at all). + from xy import ( + animation, + area, + arrow, + bar, + box, + callout, + colorbar, + column, + contour, + ecdf, + error_band, + errorbar, + export_config, + heatmap, + hexbin, + histogram, + hline, + interaction_config, + label, + legend, + line, + marker, + modebar, + r_axis, + scatter, + segments, + spring, + stairs, + stem, + step, + text, + theme, + theta_axis, + threshold, + threshold_zone, + tooltip, + violin, + vline, + x_axis, + x_band, + y_axis, + y_band, + ) + from .app import XYPlugin, append, clear_selection, reset_view, select, set_view, setup from .data_vars import AsyncDataVar, DataVar, data from .events import ( @@ -395,7 +443,20 @@ def release(token: "str | FigureHandle") -> None: SelectionPayload, ViewChangeEvent, ) - from .factories import bar_chart, chart, histogram_chart, line_chart, scatter_chart + from .factories import ( + area_chart, + bar_chart, + chart, + column_chart, + error_band_chart, + errorbar_chart, + histogram_chart, + line_chart, + scatter_chart, + segments_chart, + stem_chart, + step_chart, + ) from .handles import DataHandle, FigureHandle from .namespace import XY_NAMESPACE, XYNamespace from .registry import FigureRegistry, registry diff --git a/python/reflex_xy/app.py b/python/reflex_xy/app.py index e5153d8e..52ab1d10 100644 --- a/python/reflex_xy/app.py +++ b/python/reflex_xy/app.py @@ -13,7 +13,8 @@ What setup does: registers the `/_xy` socket.io namespace on the app's existing AsyncServer (same physical websocket as the app plane — see namespace.py), wires publish fan-out, and adds a lifespan task that -captures the event loop (for thread-safe broadcasts from sync handlers) +registers this worker's chart plans (`_ensure_page_plans`, fail-closed), +captures the event loop (for thread-safe broadcasts from sync handlers), and runs the registry TTL sweep. """ @@ -21,6 +22,7 @@ import asyncio import contextlib +from collections.abc import Coroutine from typing import Any, Optional from reflex.plugins import Plugin @@ -59,13 +61,17 @@ def setup(app: Any) -> XYNamespace: sio.register_namespace(namespace) wire(namespace) - async def _lifespan() -> None: - # The lifespan runs at server startup: every page has been added and - # the serving loop exists, in *every* worker — including backend-only - # workers that never ran the frontend compile. Register the plans - # there before serving (see _ensure_page_plans), then run the sweep. + def _lifespan() -> Coroutine[Any, Any, None]: + # Deliberately a *sync* function returning the sweep coroutine, not an + # `async def`. Reflex starts a coroutine lifespan task with + # `asyncio.create_task(task())` and then yields, so anything raised + # inside an `async def` body surfaces in the background *after* the + # worker is already serving — exactly the fail-open shape + # `_ensure_page_plans` exists to prevent. Reflex calls `task()` inline + # to *get* that coroutine, before create_task and before the lifespan + # yields, so raising from this body aborts startup instead. _ensure_page_plans(app) - await _xy_lifespan() + return _xy_lifespan() app.register_lifespan_task(_lifespan) _namespace = namespace @@ -91,7 +97,11 @@ def _ensure_page_plans(app: Any) -> None: which worker answers, with only a startup warning to explain it. Every failing page is collected and the worker refuses to start, naming the pages; the same page code already fails `reflex run`'s real compile, so - a healthy deployment never hits this. + a healthy deployment never hits this. "Refuses to start" is load-bearing + and depends on *where* this runs: `setup`'s lifespan calls it in the + synchronous part of the task, before Reflex schedules the sweep + coroutine, so the exception aborts lifespan startup rather than landing + in a background task on an already-serving worker. """ pages = getattr(app, "_unevaluated_pages", None) or {} failures: list[str] = [] diff --git a/python/reflex_xy/factories.py b/python/reflex_xy/factories.py index 0225b6d9..fed70504 100644 --- a/python/reflex_xy/factories.py +++ b/python/reflex_xy/factories.py @@ -35,6 +35,8 @@ hand-listed: positional mark params are channels, keyword-only params are options (xy's uniform convention), `Chart.__init__` provides the chart-level names, and a small reserved set belongs to the component. +Underscore-prefixed signature params are xy-internal adapter knobs and are +excluded — derivation must not promote private names to public API. Colliding mark options get flat aliases (`stroke_width` for `line`'s `width`, `mark_` otherwise); the resulting table is pinned by `tests/reflex_adapter/test_factories.py` as public contract. Unknown @@ -193,7 +195,16 @@ def known(self) -> frozenset[str]: def _flat_kind(chart_kind: str, mark_factory: Any) -> _FlatKind: - params = frozenset(inspect.signature(mark_factory).parameters) - {"data"} + # Underscore-prefixed params are xy's private adapter knobs (the pyplot + # shim's `_artist_alpha`, `_marker_path`, …), not documented mark options. + # Deriving the partition from the signature must not promote them to the + # public flat surface — they would be silently accepted *and* offered as + # did-you-mean suggestions, which is the opposite of the strict contract. + params = frozenset( + name + for name in inspect.signature(mark_factory).parameters + if name != "data" and not name.startswith("_") + ) return _FlatKind( chart_kind=chart_kind, mark_factory=mark_factory, diff --git a/scripts/bench_reflex_plans.py b/scripts/bench_reflex_plans.py index 6803fe04..09393863 100644 --- a/scripts/bench_reflex_plans.py +++ b/scripts/bench_reflex_plans.py @@ -6,7 +6,10 @@ page). Both must stay compile-scale (milliseconds), and a column republish must stay dominated by the figure build it fans out, or the tier's promise ("state deltas independent of data size, republish = one screen-bounded -reship") quietly erodes. This harness measures all of it reproducibly; +reship") quietly erodes. That last promise is a *scaling* claim, so +republish is measured as a sweep over data size rather than at one N: a +single number at a single N is consistent with any growth curve and reveals +none of them. This harness measures all of it reproducibly; recorded results live in spec/design/reflex-integration.md §6. Run (needs the reflex extra): @@ -27,10 +30,20 @@ from typing import Any REPEATS = 30 -REPUBLISH_POINTS = 100_000 STARTUP_PAGES = 20 CHARTS_PER_PAGE = 4 +#: Republish is measured as a *sweep*, not at one size. The property under +#: test — "state deltas are independent of data size; a republish is one +#: screen-bounded reship" — is a scaling claim, and a single datum at a +#: single N is consistent with O(N) without ever revealing it. The sweep +#: straddles `SCATTER_DENSITY_THRESHOLD` (200k) deliberately: below it every +#: republish rebuilds a full exact-marker figure, above it the density tier +#: takes over, and the two regimes have to be visible separately. Repeats +#: shrink with N to keep the whole run interactive. +REPUBLISH_SWEEP = ((10_000, 20), (100_000, 10), (1_000_000, 5), (5_000_000, 3)) +REPUBLISH_REFERENCE_POINTS = 100_000 + def _median_ms(fn: Callable[[], Any], repeats: int = REPEATS) -> float: times = [] @@ -91,8 +104,10 @@ def boot() -> None: return _median_ms(boot, repeats=5) -def bench_republish() -> float: +def _republish_at(points: int, repeats: int) -> float: """publish_columns -> dependent bind + figure build + publish, mounted.""" + import warnings + import numpy as np import xy @@ -102,19 +117,44 @@ def bench_republish() -> float: registry = FigureRegistry() plan = build_plan("scatter_chart", (xy.scatter("x", "y"),), {}) - data_token = "xyd1|bench-client-token|app.app.State|cloud" + data_token = f"xyd1|bench-client-token|app.app.State|cloud{points}" composite = build_plan_token(plan.digest, data_token) registry.subscribe(composite, "bench-sid", rebuildable=True) registry.bind_plan(data_token, plan.digest) rng = np.random.default_rng(7) - xs = rng.normal(size=REPUBLISH_POINTS) + xs = rng.normal(size=points) columns = {"x": xs, "y": xs * 0.5} def republish() -> None: registry.publish_columns(data_token, columns) - republish() # prime the mount - return _median_ms(republish, repeats=10) + with warnings.catch_warnings(): + # Above the direct soft ceiling xy renders a density surface and says + # so. That regime change is what the top of the sweep is *for*, so the + # notice is expected here rather than a finding. + warnings.filterwarnings("ignore", message=".*soft ceiling.*") + republish() # prime the mount + return _median_ms(republish, repeats=repeats) + + +def bench_republish_sweep() -> list[dict[str, float]]: + """Republish cost across data sizes, with per-million-point normalization. + + A flat `ms_per_million` column means the cost is dominated by the + per-point figure build and nothing worse than linear has crept in; a + column that climbs with N is the regression this exists to catch. + """ + rows: list[dict[str, float]] = [] + for points, repeats in REPUBLISH_SWEEP: + ms = _republish_at(points, repeats) + rows.append( + { + "points": points, + "republish_ms": round(ms, 2), + "ms_per_million": round(ms * 1e6 / points, 2), + } + ) + return rows def bench_plan_memory() -> float: @@ -140,13 +180,18 @@ def main() -> int: parser.add_argument("--json", action="store_true", help="machine-readable output") args = parser.parse_args() + sweep = bench_republish_sweep() + reference = next( + (row for row in sweep if row["points"] == REPUBLISH_REFERENCE_POINTS), sweep[0] + ) results = { "plan_build_ms": round(bench_plan_build(), 3), "worker_startup_ms": round(bench_worker_startup(), 1), "worker_startup_pages": STARTUP_PAGES, "worker_startup_charts": STARTUP_PAGES * CHARTS_PER_PAGE, - "republish_ms": round(bench_republish(), 1), - "republish_points": REPUBLISH_POINTS, + "republish_sweep": sweep, + "republish_ms": reference["republish_ms"], + "republish_points": reference["points"], "plan_memory_bytes": round(bench_plan_memory()), } if args.json: @@ -157,10 +202,12 @@ def main() -> int: f"worker startup page evaluation {results['worker_startup_ms']:8.1f} ms " f"({STARTUP_PAGES} pages x {CHARTS_PER_PAGE} charts)" ) - print( - f"column republish -> new payload {results['republish_ms']:8.1f} ms " - f"({REPUBLISH_POINTS:,} points, mounted)" - ) + print("column republish -> new payload (mounted, median):") + for row in sweep: + print( + f" {int(row['points']):>10,} points {row['republish_ms']:8.2f} ms " + f"({row['ms_per_million']:.2f} ms / 1M points)" + ) print(f"plan map entry {results['plan_memory_bytes']:8.0f} bytes/plan") return 0 diff --git a/spec/design/reflex-integration.md b/spec/design/reflex-integration.md index e7ea92b7..6b057b57 100644 --- a/spec/design/reflex-integration.md +++ b/spec/design/reflex-integration.md @@ -487,7 +487,16 @@ Failure is **fail-closed**: a page that cannot evaluate would leave this worker's plan map incomplete — behind a load balancer that means charts blank or not depending on which worker answers — so `_ensure_page_plans` collects every failing page and refuses worker startup with an error -naming them, instead of serving inconsistently. The contract this puts on +naming them, instead of serving inconsistently. "Refuses worker startup" +depends on *where* the pass runs: Reflex starts a coroutine lifespan task +with `asyncio.create_task(task())` and then yields, so an `async def` +lifespan body would raise in the background on an already-serving worker — +fail-open, the shape this exists to prevent. The registered task is +therefore a plain function that runs `_ensure_page_plans` synchronously and +*returns* the sweep coroutine; the raise happens in the `task()` call, in +Reflex's startup path, before serving. Pinned by +`test_page_plan_registration.py::test_page_evaluation_runs_before_the_lifespan_coroutine_is_scheduled`. +The contract this puts on app code is **re-evaluability**: a page body runs at least twice per process (the compile, then this pass) and must build the same charts each time. Code that mutates module state a later page body reads — the docs @@ -514,7 +523,12 @@ surviving to hydrate). The kwarg partition between mark options, chrome, component props, and event handlers is derived from `inspect.signature` at import — not hand-listed — so it cannot drift from xy's own signatures; collisions get generated aliases (`mark_`, with `width` becoming -`stroke_width` where the mark hasn't claimed it), pinned by test. Top-level +`stroke_width` where the mark hasn't claimed it), pinned by test. Derivation +is filtered on one axis only: underscore-prefixed signature params are xy's +private adapter knobs (the pyplot shim's `_artist_alpha`, `_marker_path`, …) +and are excluded from both the accepted set and the did-you-mean +candidates — deriving the public surface from signatures must not promote +private names into it. Top-level kwargs are **strict**: an unknown name always raises `TypeError` at page evaluation — with a did-you-mean when a known name is close — and never silently becomes a CSS property (the R8 hazard, closed rather than @@ -909,12 +923,36 @@ Recorded 2026-08-06 (Linux, Python 3.12, native core, dev machine): |---|---|---| | plan build (compile + probe + digest) | 0.26 ms median | per chart factory call, milliseconds — page evaluation stays compile-scale | | worker startup page evaluation | 15 ms (20 pages × 4 charts) | `_ensure_page_plans` re-runs pages once per worker boot; linear in charts | -| column republish → new mounted payload | 1.0 ms (100k points) | dominated by the dependent figure build, not registry bookkeeping | +| column republish → new mounted payload | see the sweep below | dominated by the dependent figure build, not registry bookkeeping | | plan map entry | ~1.5 KB/plan | process-lifetime map, bounded by page code | +Republish is recorded as a **sweep**, not as one number: "state deltas are +independent of data size, a republish is one screen-bounded reship" is a +scaling claim, and a single measurement at a single N is consistent with +every growth curve while revealing none of them. The sweep straddles +`SCATTER_DENSITY_THRESHOLD` (200k) so both regimes are visible — below it +each republish rebuilds a full exact-marker figure, above it the density +tier takes over — and the top of the range crosses the direct soft ceiling +(2M). Recorded 2026-08-06 (Linux, Python 3.11, native core, CI-class +container; a faster dev machine records ≈2× lower across the board, so the +contract is the *shape* of the column, not its constant): + +| points | republish | per 1M points | +|---|---|---| +| 10,000 | 0.37 ms | 36.8 ms | +| 100,000 | 1.47 ms | 14.7 ms | +| 1,000,000 | 3.90 ms | 3.9 ms | +| 5,000,000 | 23.6 ms | 4.7 ms | + +The normalized column falls to a floor and then stays flat: small republishes +are dominated by fixed per-publish work, large ones by the figure build they +fan out, and nothing grows faster than the build. A `per 1M points` column +that climbs with N — a second pass over the columns, a copy that used to be a +view — is the regression this sweep exists to catch. + Re-record when the probe or serialization changes materially; a plan build -drifting toward tens of milliseconds or a republish cost growing faster -than its figure build is a regression against this table. +drifting toward tens of milliseconds, or a republish cost growing faster +than its figure build, is a regression against this table. ## 7. What shipped where (prototype map) diff --git a/tests/reflex_adapter/conftest.py b/tests/reflex_adapter/conftest.py index 344a5410..d18f5257 100644 --- a/tests/reflex_adapter/conftest.py +++ b/tests/reflex_adapter/conftest.py @@ -30,6 +30,23 @@ def _fresh_registry(): adapter_app.reset_setup_for_tests() +@pytest.fixture +def app_cwd(tmp_path, monkeypatch): + """Emulate a Reflex app directory for the compile-time asset seams. + + `rx.asset` symlinks into `Path.cwd()/assets` and `payload_asset` writes + under it, so a test that mounts a chart needs a private cwd. The private + component class is cached per process and its asset registration is + per-cwd, so it is dropped too — rebuilding it exercises registration in + *this* cwd instead of reusing another test's symlinks. + """ + monkeypatch.chdir(tmp_path) + import reflex_xy.component as component_mod + + monkeypatch.setattr(component_mod, "_component_cls", None) + return tmp_path + + @pytest.fixture def client_token() -> str: return "11111111-2222-4333-8444-555566667777" diff --git a/tests/reflex_adapter/test_component.py b/tests/reflex_adapter/test_component.py index 0c490780..e6234e03 100644 --- a/tests/reflex_adapter/test_component.py +++ b/tests/reflex_adapter/test_component.py @@ -27,18 +27,6 @@ def picked(self, row: dict): ) -@pytest.fixture -def app_cwd(tmp_path, monkeypatch): - """rx.asset symlinks into Path.cwd()/assets — emulate an app directory.""" - monkeypatch.chdir(tmp_path) - # component class is cached per process; asset symlinks are per-cwd, so - # force a rebuild to exercise registration in this cwd. - import reflex_xy.component as component_mod - - monkeypatch.setattr(component_mod, "_component_cls", None) - return tmp_path - - def test_component_compiles_with_events(app_cwd): comp = reflex_xy.chart( figure=reflex_xy.FigureHandle("tok-abc"), diff --git a/tests/reflex_adapter/test_factories.py b/tests/reflex_adapter/test_factories.py index d8b44682..87f26c73 100644 --- a/tests/reflex_adapter/test_factories.py +++ b/tests/reflex_adapter/test_factories.py @@ -2,6 +2,7 @@ from __future__ import annotations +import inspect from typing import TypedDict import numpy as np @@ -38,15 +39,6 @@ def select(self, selection: dict): pass -@pytest.fixture -def app_cwd(tmp_path, monkeypatch): - monkeypatch.chdir(tmp_path) - import reflex_xy.component as component_mod - - monkeypatch.setattr(component_mod, "_component_cls", None) - return tmp_path - - def test_partition_table_is_the_public_contract(): """The generated collision table (options doc §5.6 decision 2): the component level owns the reserved names; shadowed mark options get @@ -74,6 +66,21 @@ def test_partition_table_is_the_public_contract(): } +def test_private_mark_params_stay_off_the_public_surface(app_cwd): + """The partition is signature-derived, so it inherits whatever xy's mark + signatures carry — including the pyplot shim's private adapter knobs + (`_artist_alpha`, `_marker_path`, …). Those are not documented options: + accepting them would quietly widen the flat surface, and listing them as + did-you-mean candidates would advertise it.""" + private = {name for name in inspect.signature(xy.scatter).parameters if name.startswith("_")} + assert private # the hazard is real, not hypothetical + for kind in FLAT_KINDS.values(): + assert not {name for name in kind.mark_params if name.startswith("_")} + assert not {name for name in kind.known if name.startswith("_")} + with pytest.raises(TypeError, match="_artist_alpha"): + reflex_xy.scatter_chart(data=FactoryDash.cloud, x="x", y="y", _artist_alpha=0.5) + + def test_event_trigger_set_matches_the_component(app_cwd): """EVENT_TRIGGERS is derived by hand; pin it against the real component class so the two can never drift.""" diff --git a/tests/reflex_adapter/test_page_plan_registration.py b/tests/reflex_adapter/test_page_plan_registration.py index b3b812c6..3ab3fd07 100644 --- a/tests/reflex_adapter/test_page_plan_registration.py +++ b/tests/reflex_adapter/test_page_plan_registration.py @@ -9,6 +9,7 @@ from __future__ import annotations +import inspect from types import SimpleNamespace from typing import TypedDict @@ -32,15 +33,6 @@ def table(self) -> PageSchema: return {"x": np.array([1.0]), "y": np.array([2.0])} -@pytest.fixture -def app_cwd(tmp_path, monkeypatch): - monkeypatch.chdir(tmp_path) - import reflex_xy.component as component_mod - - monkeypatch.setattr(component_mod, "_component_cls", None) - return tmp_path - - def test_backend_worker_page_evaluation_registers_plans(app_cwd, _fresh_registry): def index() -> rx.Component: return reflex_xy.scatter_chart(data=PagePlanState.table, x="x", y="y") @@ -83,3 +75,56 @@ def also_broken() -> rx.Component: def test_apps_without_unevaluated_pages_are_a_noop(): _ensure_page_plans(SimpleNamespace()) # nothing to do, nothing raised assert not _PLANS + + +def _fake_app(pages: dict, tasks: list) -> SimpleNamespace: + """Enough of an `rx.App` for `setup()`: a socket server and task sink.""" + + class _Sio: + def register_namespace(self, namespace) -> None: + pass + + return SimpleNamespace( + sio=_Sio(), + _unevaluated_pages=pages, + register_lifespan_task=tasks.append, + _state=None, + state_manager=None, + ) + + +def test_page_evaluation_runs_before_the_lifespan_coroutine_is_scheduled(app_cwd, _fresh_registry): + """Fail-closed only holds if the refusal reaches Reflex's startup path. + + Reflex runs a coroutine lifespan task as `asyncio.create_task(task())` + and then yields — so an `async def` body raising after that point would + leave the worker serving with an incomplete plan map, the exact + load-balancer-dependent failure `_ensure_page_plans` exists to prevent. + The registered task must therefore do the page pass in its *synchronous* + part (the `task()` call) and return the sweep coroutine. + """ + + def broken() -> rx.Component: + raise RuntimeError("page body exploded") + + tasks: list = [] + reflex_xy.setup(_fake_app({"broken": SimpleNamespace(component=broken)}, tasks)) + (task,) = tasks + assert not inspect.iscoroutinefunction(task) # else the raise lands in the task + with pytest.raises(RuntimeError, match="page body exploded"): + task() # Reflex calls this inline, before create_task and before yield + + +def test_lifespan_task_returns_the_sweep_coroutine(app_cwd, _fresh_registry): + """The healthy path still hands Reflex a coroutine to run as the task.""" + + def index() -> rx.Component: + return reflex_xy.scatter_chart(data=PagePlanState.table, x="x", y="y") + + tasks: list = [] + reflex_xy.setup(_fake_app({"index": SimpleNamespace(component=index)}, tasks)) + (task,) = tasks + coro = task() + assert inspect.iscoroutine(coro) + coro.close() # never awaited here; the sweep runs forever + assert len(_PLANS) == 1 # the page pass already ran, synchronously diff --git a/tests/reflex_adapter/test_payload_asset.py b/tests/reflex_adapter/test_payload_asset.py index e53a2cca..d46b2ee7 100644 --- a/tests/reflex_adapter/test_payload_asset.py +++ b/tests/reflex_adapter/test_payload_asset.py @@ -22,15 +22,6 @@ def make_chart(n: int = 32, seed: float = 1.0): return xy.line_chart(xy.line(xs, xs * seed), width=400, height=200) -@pytest.fixture -def app_cwd(tmp_path, monkeypatch): - monkeypatch.chdir(tmp_path) - import reflex_xy.component as component_mod - - monkeypatch.setattr(component_mod, "_component_cls", None) - return tmp_path - - def test_payload_asset_writes_decodable_frame(app_cwd): url = payload_asset(make_chart()) assert url.startswith("/xy/") and url.endswith(".xyf") diff --git a/tests/reflex_adapter/test_public_surface.py b/tests/reflex_adapter/test_public_surface.py new file mode 100644 index 00000000..6fc5843c --- /dev/null +++ b/tests/reflex_adapter/test_public_surface.py @@ -0,0 +1,72 @@ +"""`reflex_xy`'s public surface must be statically visible, not just lazy. + +The package resolves every export through ``__getattr__`` (an explicit +``_EXPORTS`` map plus the curated ``_XY_REEXPORTS`` set) so ``import +reflex_xy`` stays cheap. A type checker cannot follow that hook: to a +consumer, an export that is only reachable dynamically is missing or +``Any``, which silently drops the typed signatures the data-bound API is +sold on. Every name in ``__all__`` therefore also needs a static +declaration — a ``TYPE_CHECKING`` import, or a real module-level +definition. +""" + +from __future__ import annotations + +import ast +from pathlib import Path + +import pytest + +reflex_xy = pytest.importorskip("reflex_xy") + +INIT_PATH = Path(reflex_xy.__file__) + + +def _statically_declared(tree: ast.Module) -> set[str]: + """Names a type checker can see without executing ``__getattr__``.""" + declared: set[str] = set() + for statement in tree.body: + if isinstance(statement, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + declared.add(statement.name) + elif isinstance(statement, ast.AnnAssign) and isinstance(statement.target, ast.Name): + declared.add(statement.target.id) + elif isinstance(statement, ast.Assign): + declared.update( + target.id for target in statement.targets if isinstance(target, ast.Name) + ) + elif ( + isinstance(statement, ast.If) + and isinstance(statement.test, ast.Name) + and statement.test.id == "TYPE_CHECKING" + ): + for child in statement.body: + if isinstance(child, ast.ImportFrom): + declared.update( + alias.asname or alias.name for alias in child.names if alias.name != "*" + ) + elif isinstance(child, ast.Import): + declared.update( + alias.asname or alias.name.split(".", 1)[0] for alias in child.names + ) + return declared + + +def test_every_public_export_is_statically_typed(): + tree = ast.parse(INIT_PATH.read_text(encoding="utf-8"), filename=str(INIT_PATH)) + missing = sorted(set(reflex_xy.__all__) - _statically_declared(tree)) + assert not missing, ( + "reflex_xy public names have no static TYPE_CHECKING import or " + f"definition (they type as Any/missing for consumers): {missing}" + ) + + +def test_every_public_export_actually_resolves(): + """The mirror check: a static declaration with no runtime route behind it + is a typed name that fails at import.""" + unresolvable = [] + for name in reflex_xy.__all__: + try: + getattr(reflex_xy, name) + except AttributeError: # noqa: PERF203 - one report per broken name + unresolvable.append(name) + assert not unresolvable From c30f0e7588c83cea0731e1d4779aeb3deb554b31 Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Fri, 7 Aug 2026 00:13:24 +0000 Subject: [PATCH 6/6] fix(docs): version fence snapshots by page source; re-record the republish sweep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three follow-ups from review of af9f99c. Occurrence-keyed fence snapshots made a snapshot's identity positional, but `_FENCE_NAMESPACES` was never invalidated. Editing a page that repeats a fence renumbers every surviving copy, so a fence could restore a namespace captured when it sat elsewhere in the sequence — one predating the bindings now ahead of it. The dev server re-renders in-process on reload, so that is the live path. Snapshots are now versioned by a digest of the page source and a page's entries are dropped as soon as its content changes, which also bounds the cache: superseded page versions are evicted instead of holding a full namespace snapshot alive for the life of the process. The rebound-name detector only walked a fence's top-level statements, so bindings inside `for`/`with`/`if`/`try`/`match` bodies were invisible while the docstring claimed every form. It now recurses through compound statements (stopping at `def`/`class` bodies, which are a separate scope) and handles for/with/except/match targets, imports, and walrus. Imports are the substantive gain: counting them takes the guarded page set from 13 to 41 of 74, for 5.7s. The §6 republish prose claimed the normalized column "stays flat" while its own table showed 5M above the 1M floor. The table was at fault, not the prose: at one trial per size, run-to-run variance exceeds the gap between neighbouring sizes, and the 4.7 ms/1M reading was noise (three trials at that size give 3.40 / 2.74 / 3.02). The sweep now runs `REPUBLISH_TRIALS` trials per size and reports the observed range beside the median, and adds a 2M row at the direct soft ceiling. Re-recorded: 2.95 / 2.86 / 2.77 ms per 1M at 1M/2M/5M with overlapping ranges. The stated regression signal is the top of the sweep rising clear of that band, not any increase between adjacent rows. --- docs/app/tests/test_docs_site.py | 172 +++++++++++++++++++++++++----- docs/app/xy_docs/markdown.py | 42 ++++++++ scripts/bench_reflex_plans.py | 39 +++++-- spec/design/reflex-integration.md | 45 +++++--- 4 files changed, 244 insertions(+), 54 deletions(-) diff --git a/docs/app/tests/test_docs_site.py b/docs/app/tests/test_docs_site.py index 30792880..442f2ae4 100644 --- a/docs/app/tests/test_docs_site.py +++ b/docs/app/tests/test_docs_site.py @@ -33,6 +33,8 @@ ) from reflex_site_shared.docs import render_markdown from reflex_site_shared.docs.content import discover_docs +from reflex_site_shared.docs.markdown import _file_modules +from reflex_site_shared.docs.models import DocsPage from rxconfig import config from xy_docs.api_reference import ( API_REFERENCE_HEADING, @@ -1206,6 +1208,51 @@ def render() -> dict: assert second["result"]() == (1, 2) +def test_editing_a_page_drops_the_fence_snapshots_keyed_to_its_old_layout( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """A page edit must not let a fence restore a snapshot from another position. + + Snapshot keys carry the fence's occurrence index, so removing or reordering + a duplicated fence renumbers every surviving copy. Without invalidation the + renumbered fence adopts the snapshot the *other* copy left behind — a + namespace captured before the bindings that now precede it — and the page + silently renders from stale data. The dev server re-renders in-process on + reload, so this is the live path, not a hypothetical one. + """ + source_path = tmp_path / "editable.md" + + def page_of(*fences: str) -> DocsPage: + content = "\n".join(f"~~~python exec\n{body}\n~~~\n" for body in fences) + source_path.write_text(content) + return DocsPage( + source_path=source_path, + relative_path=Path("editable.md"), + route="/docs/xy/editable", + title="Editable", + description=None, + metadata={}, + content=content, + ) + + duplicated = "shared = 1" + monkeypatch.chdir(tmp_path) + + # First layout: the duplicate sits at occurrence 1, after `between`, so + # occurrence 0's snapshot is the one captured before `between` existed. + render_xy_markdown_page( + page_of(duplicated, "between = 2", duplicated, "first = (shared, between)") + ) + # The edit removes the leading copy, renumbering the survivor to occurrence + # 0 — and revises the trailing fence, as a real edit does, so it is a cache + # miss and runs against whatever namespace the restore left behind. + render_xy_markdown_page(page_of("between = 2", duplicated, "second = (shared, between)")) + + module = _file_modules[str(source_path.resolve())] + assert module.__dict__["second"] == (1, 2) + + def _assigned_names(target: ast.expr) -> set[str]: """Every top-level name an assignment target binds, unpacking included.""" if isinstance(target, ast.Name): @@ -1217,27 +1264,76 @@ def _assigned_names(target: ast.expr) -> set[str]: return set() # attribute/subscript targets rebind no module-level name -def _bound_names(body: str) -> set[str]: - """Top-level names one fence binds, in every form a demo can use. +#: Statements that run at module scope and can hold further bindings in their +#: bodies. `FunctionDef`/`ClassDef` are deliberately absent: they bind their own +#: name (handled below) but their bodies are a separate scope, so a name +#: assigned inside one is not a module-level binding and cannot shadow a +#: sibling fence's. +_NESTED_SCOPE_FREE_BODIES = ( + ast.For, + ast.AsyncFor, + ast.While, + ast.If, + ast.With, + ast.AsyncWith, + ast.Try, + ast.Match, +) - Plain assignment is not the only way to shadow a name a sibling fence - also owns: `x: list = ...`, `x += ...`, `x, y = ...`, and `def`/`class` - all do it. Matching only `ast.Assign` would skip those pages, leaving the - regression this guards unexercised on exactly the demos most likely to - hit it. - """ + +def _module_bindings(node: ast.AST) -> set[str]: + """Module-level names one statement binds, recursing into nested bodies.""" bound: set[str] = set() - for node in ast.parse(body).body: - if isinstance(node, ast.Assign): - for target in node.targets: - bound |= _assigned_names(target) - elif isinstance(node, (ast.AnnAssign, ast.AugAssign)): - bound |= _assigned_names(node.target) - elif isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): - bound.add(node.name) + if isinstance(node, ast.Assign): + for target in node.targets: + bound |= _assigned_names(target) + elif isinstance(node, (ast.AnnAssign, ast.AugAssign)): + bound |= _assigned_names(node.target) + elif isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + bound.add(node.name) # the body is another scope; the name is not + return bound + elif isinstance(node, (ast.Import, ast.ImportFrom)): + bound |= {(alias.asname or alias.name).split(".")[0] for alias in node.names} + elif isinstance(node, (ast.For, ast.AsyncFor)): + bound |= _assigned_names(node.target) + elif isinstance(node, (ast.With, ast.AsyncWith)): + for item in node.items: + if item.optional_vars is not None: + bound |= _assigned_names(item.optional_vars) + if isinstance(node, ast.Try): + bound |= {handler.name for handler in node.handlers if handler.name} + if isinstance(node, ast.Match): + for case in node.cases: + bound |= { + capture.name + for capture in ast.walk(case.pattern) + if isinstance(capture, (ast.MatchAs, ast.MatchStar)) and capture.name + } + if isinstance(node, _NESTED_SCOPE_FREE_BODIES): + for child in ast.iter_child_nodes(node): + if isinstance(child, ast.stmt): + bound |= _module_bindings(child) + # Walrus binds at the enclosing scope wherever the expression appears. + for inner in ast.walk(node): + if isinstance(inner, ast.NamedExpr): + bound |= _assigned_names(inner.target) return bound +def _bound_names(body: str) -> set[str]: + """Every module-level name one fence binds. + + Plain assignment is not the only way to shadow a name a sibling fence also + owns: annotated, augmented and unpacked assignment, `def`/`class`, imports, + `for`/`with`/`except`/`match` targets, and walrus all do it — and the + compound statements among them nest, so a binding can sit inside a + top-level `for` or `if` body. Missing any of those forms under-reports + rebound names and silently drops pages from the guarded test, which then + still passes while covering less. + """ + return {name for node in ast.parse(body).body for name in _module_bindings(node)} + + def _names_rebound_across_exec_fences(content: str) -> set[str]: """Top-level names that more than one exec fence of a page binds.""" seen: set[str] = set() @@ -1255,11 +1351,27 @@ def test_rebound_name_detection_covers_every_binding_form() -> None: """Pin the detector itself: the page filter above is only as complete as the binding forms it recognizes.""" fence = "~~~python exec\n{body}\n~~~" + declared = ( + "plain", + "annotated", + "augmented", + "unpacked", + "spread", + "fn", + "Cls", + "np", + "pi", + "looped", + "handle", + "caught", + "matched", + "walrus", + "nested", + ) page = "\n".join( fence.format(body=body) for body in ( - "plain = 1\nannotated: int = 1\naugmented = 0\nunpacked = 0\nspread = 0\n" - "def fn():\n pass\n\n\nclass Cls:\n pass", + "\n".join(f"{name} = 0" for name in declared), "plain = 2", "annotated: int = 2", "augmented += 2", @@ -1267,18 +1379,22 @@ def test_rebound_name_detection_covers_every_binding_form() -> None: "*spread, tail = [2, 3]", "def fn():\n return 2", "class Cls:\n pass", - "obj.attr = 2\nmapping['key'] = 2", # rebinds no module-level name + "import numpy as np", + "from math import pi", + "for looped in range(2):\n pass", + "with open(__file__) as handle:\n pass", + "try:\n pass\nexcept ValueError as caught:\n pass", + "match [1]:\n case [matched]:\n pass", + "if (walrus := 2):\n pass", + # A binding inside a top-level compound body is still module scope. + "for _ in range(1):\n if True:\n nested = 2", + # Neither rebinds a module-level name: attribute/subscript targets, + # and names assigned inside a function body (a separate scope). + "obj.attr = 2\nmapping['key'] = 2", + "def _unrelated():\n plain = 3\n return plain", ) ) - assert _names_rebound_across_exec_fences(page) == { - "plain", - "annotated", - "augmented", - "unpacked", - "spread", - "fn", - "Cls", - } + assert _names_rebound_across_exec_fences(page) == set(declared) def test_repeated_page_renders_rebuild_every_demo_from_its_own_data( diff --git a/docs/app/xy_docs/markdown.py b/docs/app/xy_docs/markdown.py index 90a747a2..b44bdc15 100644 --- a/docs/app/xy_docs/markdown.py +++ b/docs/app/xy_docs/markdown.py @@ -2,6 +2,7 @@ from __future__ import annotations +import hashlib import json from collections import Counter from dataclasses import replace @@ -42,6 +43,40 @@ # See `_exec_fence`. _FENCE_NAMESPACES: dict[tuple[str, str, int], dict] = {} +# Digest of the page source each filepath's snapshots were captured from. A +# snapshot's key includes the fence's *position*, so entries are only valid for +# the exact page content that produced them. See `_invalidate_stale_fences`. +_FENCE_PAGE_DIGESTS: dict[str, str] = {} + + +def _invalidate_stale_fences(virtual_filepath: str, content: str) -> None: + """Drop a page's fence snapshots when its source no longer matches theirs. + + Snapshots are keyed by (filepath, fence source, occurrence index), so a + fence's identity depends on where it sits in the page's fence sequence. + Within one process that is exactly right — the repeat renders this cache + exists for (frontend compile, then `reflex_xy`'s worker-startup pass) walk + identical content. Across an edit it is not: adding, removing or reordering + a duplicated fence shifts the occurrence index of every surviving copy, so + a fence could restore a snapshot captured when it sat at a different + position, silently reviving a namespace that predates the bindings now + ahead of it. The dev server re-renders in-process on reload, which is + precisely where that happens. + + So version the cache by page source: whenever a page renders with content + that differs from the digest its entries were captured under, those entries + are stale by construction and get dropped. This also bounds the cache — + superseded page versions are evicted rather than retained for the life of + the process, each one holding a full namespace snapshot (State classes, + arrays, every binding) alive. + """ + digest = hashlib.sha256(content.encode()).hexdigest() + if _FENCE_PAGE_DIGESTS.get(virtual_filepath) == digest: + return + _FENCE_PAGE_DIGESTS[virtual_filepath] = digest + for key in [key for key in _FENCE_NAMESPACES if key[0] == virtual_filepath]: + del _FENCE_NAMESPACES[key] + def _split_demo_data(content: str) -> tuple[str | None, str]: """Split a demo fence into (data, code) around ``_DEMO_DATA_DIVIDER``. @@ -173,6 +208,10 @@ def _exec_fence(self, content: str) -> None: discarding whatever the fences in between defined, which is not what the shared renderer does (it skips re-execution and lets the page's namespace keep accumulating). + + Because that identity is positional, it is only meaningful for the page + source it was captured from; `_invalidate_stale_fences` drops a page's + entries as soon as its content changes. """ occurrence = self.fence_occurrences[content] self.fence_occurrences[content] += 1 @@ -230,6 +269,9 @@ def _render_demo(self, content: str, flags: set[str]) -> rx.Component: def render_xy_markdown_page(page: DocsPage) -> rx.Component: """Render one discovered XY documentation page.""" source_path = page.source_path.resolve() + # Cached snapshots are keyed by fence position, so they only survive while + # the page source that produced them does. + _invalidate_stale_fences(str(source_path), page.content) # One fence sequence per page render, even though the body and the FAQ are # transformed separately around the generated API section. fence_occurrences: Counter[str] = Counter() diff --git a/scripts/bench_reflex_plans.py b/scripts/bench_reflex_plans.py index 09393863..dc33a6ad 100644 --- a/scripts/bench_reflex_plans.py +++ b/scripts/bench_reflex_plans.py @@ -41,9 +41,23 @@ #: republish rebuilds a full exact-marker figure, above it the density tier #: takes over, and the two regimes have to be visible separately. Repeats #: shrink with N to keep the whole run interactive. -REPUBLISH_SWEEP = ((10_000, 20), (100_000, 10), (1_000_000, 5), (5_000_000, 3)) +REPUBLISH_SWEEP = ( + (10_000, 25), + (100_000, 15), + (1_000_000, 9), + (2_000_000, 7), + (5_000_000, 5), +) REPUBLISH_REFERENCE_POINTS = 100_000 +#: Each size is measured over several independent trials, and the sweep +#: reports the spread across them alongside the median. Run-to-run variance at +#: the top of the sweep is comparable to the differences between neighbouring +#: sizes, so a single median per size cannot distinguish a real trend from +#: noise — and reading a lone high point as a trend is exactly the mistake the +#: recorded table is there to prevent. +REPUBLISH_TRIALS = 3 + def _median_ms(fn: Callable[[], Any], repeats: int = REPEATS) -> float: times = [] @@ -140,18 +154,24 @@ def republish() -> None: def bench_republish_sweep() -> list[dict[str, float]]: """Republish cost across data sizes, with per-million-point normalization. - A flat `ms_per_million` column means the cost is dominated by the - per-point figure build and nothing worse than linear has crept in; a - column that climbs with N is the regression this exists to catch. + `ms_per_million` settling into a band as N grows means the cost is + dominated by the per-point figure build and nothing worse than linear has + crept in. The regression this exists to catch is that normalized value + climbing *clear of the band* at the top of the sweep — `ms_per_million_min` + and `_max` are reported so the band is visible and a single high median is + not mistaken for a trend. """ rows: list[dict[str, float]] = [] for points, repeats in REPUBLISH_SWEEP: - ms = _republish_at(points, repeats) + trials = [_republish_at(points, repeats) for _ in range(REPUBLISH_TRIALS)] + per_million = sorted(ms * 1e6 / points for ms in trials) rows.append( { "points": points, - "republish_ms": round(ms, 2), - "ms_per_million": round(ms * 1e6 / points, 2), + "republish_ms": round(statistics.median(trials), 2), + "ms_per_million": round(statistics.median(per_million), 2), + "ms_per_million_min": round(per_million[0], 2), + "ms_per_million_max": round(per_million[-1], 2), } ) return rows @@ -202,11 +222,12 @@ def main() -> int: f"worker startup page evaluation {results['worker_startup_ms']:8.1f} ms " f"({STARTUP_PAGES} pages x {CHARTS_PER_PAGE} charts)" ) - print("column republish -> new payload (mounted, median):") + print(f"column republish -> new payload (mounted, median of {REPUBLISH_TRIALS} trials):") for row in sweep: print( f" {int(row['points']):>10,} points {row['republish_ms']:8.2f} ms " - f"({row['ms_per_million']:.2f} ms / 1M points)" + f"({row['ms_per_million']:5.2f} ms / 1M points, " + f"{row['ms_per_million_min']:.2f}-{row['ms_per_million_max']:.2f} across trials)" ) print(f"plan map entry {results['plan_memory_bytes']:8.0f} bytes/plan") return 0 diff --git a/spec/design/reflex-integration.md b/spec/design/reflex-integration.md index 6b057b57..f52a536a 100644 --- a/spec/design/reflex-integration.md +++ b/spec/design/reflex-integration.md @@ -932,23 +932,34 @@ scaling claim, and a single measurement at a single N is consistent with every growth curve while revealing none of them. The sweep straddles `SCATTER_DENSITY_THRESHOLD` (200k) so both regimes are visible — below it each republish rebuilds a full exact-marker figure, above it the density -tier takes over — and the top of the range crosses the direct soft ceiling -(2M). Recorded 2026-08-06 (Linux, Python 3.11, native core, CI-class -container; a faster dev machine records ≈2× lower across the board, so the -contract is the *shape* of the column, not its constant): - -| points | republish | per 1M points | -|---|---|---| -| 10,000 | 0.37 ms | 36.8 ms | -| 100,000 | 1.47 ms | 14.7 ms | -| 1,000,000 | 3.90 ms | 3.9 ms | -| 5,000,000 | 23.6 ms | 4.7 ms | - -The normalized column falls to a floor and then stays flat: small republishes -are dominated by fixed per-publish work, large ones by the figure build they -fan out, and nothing grows faster than the build. A `per 1M points` column -that climbs with N — a second pass over the columns, a copy that used to be a -view — is the regression this sweep exists to catch. +tier takes over — and the range spans the direct soft ceiling (2M). Each size +is measured over `REPUBLISH_TRIALS` independent trials, because at the top of +the sweep run-to-run variance is comparable to the gap between neighbouring +sizes: a single median per size cannot tell a trend from noise, and reading +one high point as a trend is precisely the error this table must not invite. +Recorded 2026-08-06 (Linux, Python 3.11, native core, CI-class container; a +faster dev machine records lower across the board, so the contract is the +*shape* of the normalized column, not its constant): + +| points | republish | per 1M points | per 1M across trials | +|---|---|---|---| +| 10,000 | 0.23 ms | 22.81 ms | 21.54 – 24.78 | +| 100,000 | 1.22 ms | 12.22 ms | 12.09 – 12.47 | +| 1,000,000 | 2.95 ms | 2.95 ms | 2.94 – 3.06 | +| 2,000,000 | 5.72 ms | 2.86 ms | 2.78 – 2.87 | +| 5,000,000 | 13.83 ms | 2.77 ms | 2.65 – 2.90 | + +The normalized column falls steeply while fixed per-publish work still +dominates, then settles into a band — ~2.7–3.1 ms/1M from 1M up, with the +three large sizes' trial ranges overlapping. Above the threshold the cost is +the per-point figure build the republish fans out, and nothing is growing +faster than it. + +The regression signal is the normalized value at the top of the sweep rising +*clear of that band* — a second pass over the columns, a copy that used to be +a view — not any increase between adjacent rows. Neighbouring sizes here +differ by less than the spread within a single size, so a one-row uptick is +noise until a trial range separates it. Compare bands, not medians. Re-record when the probe or serialization changes materially; a plan build drifting toward tens of milliseconds, or a republish cost growing faster