Skip to content

fix(config): preserve preloaded rxconfig classes - #7144

Open
FarhanAliRaza wants to merge 3 commits into
reflex-dev:mainfrom
FarhanAliRaza:codex/config-reload-boundary
Open

FarhanAliRaza wants to merge 3 commits into
reflex-dev:mainfrom
FarhanAliRaza:codex/config-reload-boundary

Conversation

@FarhanAliRaza

@FarhanAliRaza FarhanAliRaza commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Reuse the current project's already-imported rxconfig configuration instead of re-executing it during app initialization.
  • Remove the redundant reflex run config reload after port and host overrides are already persisted.
  • Isolate app/config unit-test context state and cover direct rxconfig classes, pickle identity, and cross-project loading.

Validation

  • 327 focused unit tests passed.
  • Ruff and pyright passed.
  • A real Reflex app passed production export, dev startup, State events, pickle roundtrip, and an rxconfig.py hot reload.

Review in cubic

@FarhanAliRaza
FarhanAliRaza requested a review from a team as a code owner September 14, 2026 21:59
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-14T22:03:46.895468Z fe952bc PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@greptile-apps

greptile-apps Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

The behavioral fix appears safe, but the explicit repository requirement against hardcoded identifier literals must still be satisfied before merging.

Summary

The PR preserves classes defined in a preloaded rxconfig.py by reusing the active registration context’s configuration rather than re-executing the module. It also routes backend servers through a config-first app factory and removes a redundant CLI reload.

  • Reuses cached configuration during App construction and backend app loading.
  • Preserves class and app identity in forked workers with regression coverage.
  • Isolates app and config slots between unit tests.
  • Removes the redundant run-command configuration reload.

Reviews (3) · Last reviewed commit: "fix(config): preserve inherited config i..."

Comment thread packages/reflex-base/src/reflex_base/config.py Outdated
Comment thread tests/units/test_app.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fe952bca61

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/reflex-base/src/reflex_base/config.py Outdated
Comment thread packages/reflex-base/src/reflex_base/config.py Outdated
@codspeed-hq

codspeed-hq Bot commented Sep 14, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 4.78%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 1 regressed benchmark
✅ 39 untouched benchmarks
⏩ 8 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
test_collect_imports[_complicated_page] 3.6 ms 3.8 ms -4.78%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing FarhanAliRaza:codex/config-reload-boundary (dec1771) with main (d1a6d2e)

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 8 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/reflex-base/src/reflex_base/config.py Outdated
Comment thread packages/reflex-base/src/reflex_base/config.py Outdated
Comment thread tests/units/test_app.py
Comment thread reflex/utils/exec.py Outdated
Comment thread reflex/utils/exec.py

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 6 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="tests/units/utils/test_exec.py">

<violation number="1" location="tests/units/utils/test_exec.py:78">
P2: This test loads rxconfig.py whose `rx.Config(...)` sets the module global `reflex_base.config._state_auto_setters`, but the finally block only restores `_config_module_deps`/`_config_module_deps_root`. The leaked value stays cached for later tests, overriding the env-var fallback in `get_state_auto_setters()` and any State created without a fresh config load. Save `_state_auto_setters` before the load and restore it in `finally`, mirroring tests/units/test_app.py:136-154.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

sys.modules.pop(config_module_name, None)
sys.modules.pop("config_first_app", None)
sys.modules.pop("config_first_app.config_first_app", None)
reflex_base.config._config_module_deps.clear()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This test loads rxconfig.py whose rx.Config(...) sets the module global reflex_base.config._state_auto_setters, but the finally block only restores _config_module_deps/_config_module_deps_root. The leaked value stays cached for later tests, overriding the env-var fallback in get_state_auto_setters() and any State created without a fresh config load. Save _state_auto_setters before the load and restore it in finally, mirroring tests/units/test_app.py:136-154.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/units/utils/test_exec.py, line 78:

<comment>This test loads rxconfig.py whose `rx.Config(...)` sets the module global `reflex_base.config._state_auto_setters`, but the finally block only restores `_config_module_deps`/`_config_module_deps_root`. The leaked value stays cached for later tests, overriding the env-var fallback in `get_state_auto_setters()` and any State created without a fresh config load. Save `_state_auto_setters` before the load and restore it in `finally`, mirroring tests/units/test_app.py:136-154.</comment>

<file context>
@@ -3,18 +3,82 @@
+        sys.modules.pop(config_module_name, None)
+        sys.modules.pop("config_first_app", None)
+        sys.modules.pop("config_first_app.config_first_app", None)
+        reflex_base.config._config_module_deps.clear()
+        reflex_base.config._config_module_deps_root = None
+
</file context>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant