Data-bound chart components 6/7: compile probe for figure builders - #466
Data-bound chart components 6/7: compile probe for figure builders#466FarhanAliRaza wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Greptile SummaryThe PR adds compile-time validation for
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| python/reflex_xy/app.py | Adds state-tree traversal, builder probing, diagnostic wrapping, and plugin integration; no follow-up-eligible issue was established. |
| python/reflex_xy/vars.py | Adds the public probe option, validates accepted levels, selects sync/async defaults, and stores probe metadata on the computed-var getter. |
| python/reflex_xy/tokens.py | Defines the getter attribute used to preserve and retrieve compile-probe configuration. |
| spec/design/reflex-integration.md | Documents probe levels, defaults, failure behavior, and the session-dependent warning escape valve. |
| tests/reflex_adapter/test_figure_probe.py | Covers core probing behavior, opt-outs, diagnostics, async opt-in, session-dependent warnings, and invalid configuration. |
Reviews (2): Last reviewed commit: "feat(reflex): compile-time probe for @re..." | Re-trigger Greptile
Merging this PR will not alter performance
Comparing 
Footnotes
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/reflex_adapter/test_figure_probe.py">
<violation number="1" location="tests/reflex_adapter/test_figure_probe.py:74">
P2: These tests probe the whole rx.State tree rather than scoping to ProbeDemo. That ties their outcome to every other probe-enabled figure builder registered anywhere in the pytest session: any unrelated module's builder that isn't a no-op under default state (or that the process-wide walk reaches) would fail or downgrade these tests. Scope the probe to the test state, e.g. probe_figure_builders(ProbeDemo), so each test is self-contained and isolation is explicit.</violation>
</file>
<file name="python/reflex_xy/vars.py">
<violation number="1" location="python/reflex_xy/vars.py:186">
P3: Invalid probe values can slip through: `probe=0` is currently accepted because membership uses equality and `0 == False`. This weakens the new validation contract and can pass a non-supported level downstream; comparing `False` by identity avoids that edge case.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
|
||
|
|
||
| def test_probe_runs_sync_builders_and_skips_optouts(): | ||
| probed = probe_figure_builders() |
There was a problem hiding this comment.
P2: These tests probe the whole rx.State tree rather than scoping to ProbeDemo. That ties their outcome to every other probe-enabled figure builder registered anywhere in the pytest session: any unrelated module's builder that isn't a no-op under default state (or that the process-wide walk reaches) would fail or downgrade these tests. Scope the probe to the test state, e.g. probe_figure_builders(ProbeDemo), so each test is self-contained and isolation is explicit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/reflex_adapter/test_figure_probe.py, line 74:
<comment>These tests probe the whole rx.State tree rather than scoping to ProbeDemo. That ties their outcome to every other probe-enabled figure builder registered anywhere in the pytest session: any unrelated module's builder that isn't a no-op under default state (or that the process-wide walk reaches) would fail or downgrade these tests. Scope the probe to the test state, e.g. probe_figure_builders(ProbeDemo), so each test is self-contained and isolation is explicit.</comment>
<file context>
@@ -0,0 +1,113 @@
+
+
+def test_probe_runs_sync_builders_and_skips_optouts():
+ probed = probe_figure_builders()
+ names = {name.rsplit(".", 1)[-1] for name in probed if "probe_demo" in name}
+ assert "healthy" in names
</file context>
| ``async def`` builders, whose data sources should not be awaited at | ||
| compile. | ||
| """ | ||
| if probe not in (*_PROBE_LEVELS, None): |
There was a problem hiding this comment.
P3: Invalid probe values can slip through: probe=0 is currently accepted because membership uses equality and 0 == False. This weakens the new validation contract and can pass a non-supported level downstream; comparing False by identity avoids that edge case.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At python/reflex_xy/vars.py, line 186:
<comment>Invalid probe values can slip through: `probe=0` is currently accepted because membership uses equality and `0 == False`. This weakens the new validation contract and can pass a non-supported level downstream; comparing `False` by identity avoids that edge case.</comment>
<file context>
@@ -167,7 +172,20 @@ async def remote(self) -> xy.Chart:
+ ``async def`` builders, whose data sources should not be awaited at
+ compile.
"""
+ if probe not in (*_PROBE_LEVELS, None):
+ msg = f"@reflex_xy.figure probe= must be 'build', 'figure', or False, got {probe!r}"
+ raise ValueError(msg)
</file context>
| if probe not in (*_PROBE_LEVELS, None): | |
| if probe is not None and probe is not False and probe not in ("build", "figure"): | |
With the data-bound tier validating structure at page evaluation, the figure var is the last place chart-building code defers to hydrate — where a typo'd mark name or a bad kwarg shows up as a blank mount and an err frame, in a browser, after a round trip. XYPlugin.post_compile now walks the state tree and runs each figure builder once against a default state instance. probe="build" (the sync default) runs the body; probe="figure" also compiles the result; probe=False opts out and is the default for async builders, because awaiting a data source at compile is exactly what the "no data ingestion at compile" constraint forbids — an async builder can still opt in. Failures raise FigureProbeError naming the state class, var, and source location, wrapping the original exception. One deliberate softening: a builder whose source reads self.router is session-dependent by declaration, and only a live session can validate it, so its probe failure degrades to a RuntimeWarning rather than failing the compile. The heuristic is source text, which is why it only ever downgrades an error — never invents one. Spec: reflex-integration.md §3.1 (compile probe).
1411f82 to
2641bf4
Compare
1 participant
Stacked on #465. Base is
stack/5-chart-factories.Why
With the data-bound tier validating structure at page evaluation,
@reflex_xy.figureis the last place chart-building code defers to hydrate — where a typo'd mark name or a bad kwarg shows up as a blank mount and anerrframe, in a browser, after a round trip.Change
XYPlugin.post_compilewalks the state tree and runs each figure builder once against a default state instance:probe="build"(sync default) — run the body;probe="figure"— also compile the result;probe=False— opt out, and the default forasync defbuilders, because awaiting a data source at compile is exactly what the "no data ingestion at compile" constraint forbids. An async builder can still opt in explicitly.Failures raise
FigureProbeErrornaming the state class, var, and source location, wrapping the original exception.One deliberate softening
A builder whose source reads
self.routeris session-dependent by declaration, and only a live session can validate it — so its probe failure degrades to aRuntimeWarningrather than failing the compile. The heuristic is source text, which is why it can only ever downgrade an error, never invent one.Spec
reflex-integration.md§3.1 (compile probe).Test plan
uv run pytest tests/reflex_adapter tests/test_validation_timing.py— 240 passedpre-commit run --all-files,ruff check,ruff format --check,ty check— clean