Repository navigation
Bound and test Reflex compatibility window - #475
harsh21234i wants to merge 10 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Compatibility contract and release policy pyproject.toml, scripts/artifact_metadata.py, docs/overview/installation.md, docs/advanced/runtime-and-deployment.md, spec/design/reflex-integration.md, spec/process/production-readiness.md, tests/test_artifact_metadata.py, tests/test_verify_sdist.py, tests/test_verify_wheel.py |
The Reflex extra and artifact metadata specify >=0.9.6,<=0.10.0. Documentation describes the tested versions and release requirements. Metadata and artifact tests check the bounded requirement. |
CI compatibility matrix .github/workflows/ci.yml, scripts/verify_ci_workflow.py, tests/test_verify_ci_workflow.py |
CI runs component and state smoke tests against Reflex 0.9.6, 0.9.13, and 0.10.0. The workflow validator checks the matrix, commands, and metadata requirement. Tests cover invalid matrix entries, inactive commands, and missing bounds. |
Release smoke-test bounds .github/workflows/build_release_artifacts.yml, .github/workflows/ci.yml, scripts/verify_ci_workflow.py, tests/test_verify_ci_workflow.py |
Native-wheel and sdist smoke installs use >=0.9.6,<=0.10.0. The workflow validator and tests check the sdist install constraints. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix · Severity of issue fixed: Medium
Sequence Diagram(s)
sequenceDiagram
participant CIWorkflow
participant PackageMetadata
participant ComponentAndStateSmokes
CIWorkflow->>PackageMetadata: verify Reflex version and bounded requirement
CIWorkflow->>ComponentAndStateSmokes: run component and state tests for 0.9.6, 0.9.13, and 0.10.0
Possibly related PRs
- reflex-dev/xy#425: Adds an ordered release checklist with sdist and clean-install checks, and links it from production-readiness documentation.
Suggested reviewers: alek99, farhanaliraza
Merge Risk: 
Minimal · previous review e5fc3
-
The change bounds the Reflex extra to the versions covered by the compatibility matrix and adds checks that stop untested versions from being admitted. No concrete merge-blocking defect remains. Confirm the CI compatibility jobs pass for all three matrix versions before merging.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Linked Issues check
-
Explanation
Issue
#447requires a declared and documented tested Reflex range, minimum and highest-release compile/event smokes, rejection of untested newer releases, and a release rule for widening the range.pyproject.tomldeclaresreflex>=0.9.6,<=0.10.0. CI tests exact versions 0.9.6, 0.9.13, and 0.10.0 with component and state-rebuild smokes. The workflow verifier and artifact tests enforce the bound. The production-readiness release gate requires compatibility-matrix coverage before widening the range.
Full details: Out of Scope Changes check
-
Explanation
The changed workflow checks, artifact metadata checks, CI matrix, release checks, documentation, and tests directly support issue
#447. No unrelated change is identified in the reviewed summary or inspected files.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 12.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 6 files. (3 skipped: 3 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
⚔️ Resolve merge conflicts: 💡
-
- Resolve merge conflict in branch
fix/reflex-compatibility-window-447
- Resolve merge conflict in branch
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/verify_ci_workflow.py (1)
1153-1162: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate the bounded requirement in both coreless sdist smoke steps.
The coreless CI and release checks do not require
REFLEX_REQUIREMENT. A future change can remove the upper bound from either coreless installation command without failingscripts/verify_ci_workflow.py.
scripts/verify_ci_workflow.py#L1153-L1162: requiref'"{REFLEX_REQUIREMENT}"'in the CI coreless-sdist step.scripts/verify_ci_workflow.py#L1364-L1373: requiref'"{REFLEX_REQUIREMENT}"'in the release coreless-sdist step.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/verify_ci_workflow.py` around lines 1153 - 1162, The coreless sdist smoke-step validation must enforce the bounded REFLEX_REQUIREMENT in both locations. Update _require_step_contains at scripts/verify_ci_workflow.py lines 1153-1162 and 1364-1373 to require f'"{REFLEX_REQUIREMENT}"' for the CI and release coreless-sdist steps, respectively.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
251-253: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDisable persisted checkout credentials.
actions/checkoutstores the workflow token in local Git configuration by default. The laternpm ci, package build, and test commands can read that credential. Setpersist-credentials: false. No displayed step needs Git write access.Proposed change
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: fetch-depth: 0 + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 251 - 253, Update the actions/checkout step to set persist-credentials to false alongside fetch-depth, ensuring the checkout token is not stored in local Git configuration.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/verify_ci_workflow.py`:
- Around line 888-897: Update the project metadata validation around
REFLEX_REQUIREMENT to parse pyproject.toml as TOML and inspect
project.optional-dependencies.reflex directly. Require that the reflex extra
exists and contains exactly REFLEX_REQUIREMENT, rejecting placements in other
dependency groups, base dependencies, comments, or additional reflex
requirements while preserving the existing read-error reporting.
---
Outside diff comments:
In `@scripts/verify_ci_workflow.py`:
- Around line 1153-1162: The coreless sdist smoke-step validation must enforce
the bounded REFLEX_REQUIREMENT in both locations. Update _require_step_contains
at scripts/verify_ci_workflow.py lines 1153-1162 and 1364-1373 to require
f'"{REFLEX_REQUIREMENT}"' for the CI and release coreless-sdist steps,
respectively.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 251-253: Update the actions/checkout step to set
persist-credentials to false alongside fetch-depth, ensuring the checkout token
is not stored in local Git configuration.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 317d8e05-a228-443d-ae92-131597dd25f3
📒 Files selected for processing (6)
.github/workflows/ci.yml.github/workflows/release.ymldocs/advanced/runtime-and-deployment.mdpyproject.tomlscripts/verify_ci_workflow.pyspec/process/production-readiness.md
There was a problem hiding this comment.
All reported issues were addressed across 6 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/verify_ci_workflow.py (1)
903-911: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate the Reflex matrix exactly.
Line 903 only searches the job text. A comment can contain the expected matrix text. A matrix can also add an untested version and still pass this check.
Parse the
reflex_compatibilitymatrix. Require exactly["0.9.6", "0.9.8"]andfail-fast: false.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/verify_ci_workflow.py` around lines 903 - 911, The reflex_compatibility validation currently relies on text search and can accept comments or extra versions. Update the verification around _require_job_contains to parse that job’s matrix configuration and require exactly ["0.9.6", "0.9.8"] for reflex-version plus fail-fast: false, rejecting any additional or missing entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/verify_ci_workflow.py`:
- Around line 616-624: Update the validation around _step_run_lines and the
hard-gate step checks so required commands are recognized only when actually
executable, not when merely echoed or mentioned in uninvoked shell functions.
Parse and validate the concrete active command list, then add regression
coverage for echoed command text and uninvoked shell functions while preserving
detection of genuinely executed smoke tests.
---
Outside diff comments:
In `@scripts/verify_ci_workflow.py`:
- Around line 903-911: The reflex_compatibility validation currently relies on
text search and can accept comments or extra versions. Update the verification
around _require_job_contains to parse that job’s matrix configuration and
require exactly ["0.9.6", "0.9.8"] for reflex-version plus fail-fast: false,
rejecting any additional or missing entries.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 37160a62-4113-4e5b-af63-e67f9a8fe485
📒 Files selected for processing (5)
docs/overview/installation.mdscripts/artifact_metadata.pyscripts/verify_ci_workflow.pytests/test_artifact_metadata.pytests/test_verify_ci_workflow.py
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
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="scripts/verify_ci_workflow.py">
<violation number="1" location="scripts/verify_ci_workflow.py:617">
P1: The Reflex compatibility gate can be bypassed with heredoc or inactive-shell data: exact command text is sufficient for approval even when the shell never executes it. The command check should distinguish active shell commands from heredoc, function, and conditional bodies before accepting the gate.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
FarhanAliRaza
left a comment
There was a problem hiding this comment.
Required changes (details inline):
- Stale
uv.lock.pyproject.tomlchanges thereflexextra, but the lock was not regenerated.uv lock --checkanduv sync --locked --extra reflex --group devfail on this head, so the main CI test job will fail. Runuv lockand commit. - The matrix top is not the newest release in the window. PyPI has Reflex 0.9.9 to 0.9.13.
>=0.9.6,<0.10admits all of them untested, and #447 asks the resolver to reject untested versions. Test 0.9.13, or cap the extra below 0.9.9. Both matrix smokes pass on 0.9.13. - The
<0.10cap has no recorded reason. Reflex 0.10.0 is released. Both smokes and the fulltests/reflex_adaptersuite pass on it. Record the incompatibility in the spec, or test 0.10.0 and widen. - Spec drift.
spec/design/reflex-integration.md:725still saysreflex>=0.9.6.production-readiness.mdcalls 0.9.8 the newest release.
What I validated on the head commit (f8795b2):
- I ran the new
reflex_compatibilityjob steps exactly as written, on Reflex 0.9.6 and 0.9.8. The install, the version and metadata assertions, and both smokes pass. - I ran the
examples/reflexdemo app on Reflex 0.9.6 and 0.9.8, and drove it withscripts/reflex_ws_smoke.pyin headless Chromium. All charts paint. Density drilldown and hover readout work. The live stream appends. The probe's single-websocket assertion fails on both versions with 13 to 16 sockets. This also happens on 0.9.8, the versionmainlocks, so it is not caused by this PR. tests/test_verify_ci_workflow.pyandtests/test_artifact_metadata.pypass (102 tests).scripts/verify_ci_workflow.pypasses. Ruff check and format are clean.- Only bot checks ran on this fork PR. Repo CI has not run.
Generated by Claude Code
[Medium impact] The PR appears safe to merge; no new blocking issue was found. SummaryBounds
No new actionable issues were found. Reviews (3) · Last reviewed commit: "Merge upstream main into Reflex compatib..." · Reviewed by Greptile |
There was a problem hiding this comment.
All reported issues were addressed across 14 files
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
View guided diff | Re-trigger cubic
Preserve PR behavior while integrating the shared CI build and selection changes.
Merging this PR will not alter performance1 warning, 1 note
Comparing Footnotes
|
Fixes #447
Summary
xy[reflex]to>=0.9.6,<=0.10.0; CI samples the minimum, latest 0.9.x, and upper bound (0.9.6, 0.9.13, and 0.10.0).Testing
scripts/verify_ci_workflow.pypassed.uv lock --checkand the reflex-release workflow sync check passed.Summary by CodeRabbit
Compatibility
>=0.9.6,<=0.10.0; compatibility checks cover versions 0.9.6, 0.9.13, and 0.10.0.Documentation