Skip to content

Fix config reload state imports (#7028) - #7035

Open
harsh21234i wants to merge 8 commits into
reflex-dev:mainfrom
harsh21234i:fix/config-reload-state-imports
Open

Fix config reload state imports (#7028)#7035
harsh21234i wants to merge 8 commits into
reflex-dev:mainfrom
harsh21234i:fix/config-reload-state-imports

Conversation

@harsh21234i

@harsh21234i harsh21234i commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

• ## Summary
Fixes: #7028
Fix reload_config() raising StateValueError when rxconfig.py imports a
module that defines a rx.State class.

Changes

  • Preserve project-local dependencies during reloads within the same
    RegistrationContext.

  • Continue evicting dependencies for new contexts or different project
    roots.

  • Added regression tests for state reloads, fresh contexts, and cross-
    project isolation.

  • Added a reflex-base bugfix changelog fragment.

Testing

  • Focused regression tests: 3 passed
  • Ruff check and formatting: passed

Review in cubic

@harsh21234i
harsh21234i requested a review from a team as a code owner September 2, 2026 17:07
@greptile-apps

greptile-apps Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR changes config reload dependency tracking so project-local state modules retain their registered class identities while ordinary configuration dependencies are re-imported.

  • Stores project-local dependency module objects and their project root in each RegistrationContext.
  • Restores registered state modules during same-context reloads while refreshing non-state dependencies and package settings.
  • Records imports even when package reloading fails, allowing a later retry to evict partially loaded dependencies.
  • Adds regression coverage for state imports, context and project isolation, package settings, and failed-reload recovery.
  • Adds the required reflex-base bugfix changelog fragment.

Confidence Score: 4/5

The PR is not yet safe to merge because a package that both defines a registered state and contains another state module is still reloaded, recreating the package-level state class.

The unresolved previous finding in packages/reflex-base/src/reflex_base/config.py remains blocking: a dependency can belong to both state_modules and package_modules, but every package in package_modules is still passed to importlib.reload, so its initializer can redefine a state class already retained by the active registration context. Two other previous threads were manually resolved without explanatory replies; they do not affect merge safety.

Files Needing Attention: packages/reflex-base/src/reflex_base/config.py

Important Files Changed

Filename Overview
packages/reflex-base/src/reflex_base/config.py Implements context-aware module preservation and reload tracking; the previously reported package/state overlap remains outstanding.
packages/reflex-base/src/reflex_base/registry.py Stores and copies each registration context's config dependency modules and associated project root.
tests/units/test_config.py Adds comprehensive regression tests for dependency freshness, state identity, context isolation, package reloads, and retry behavior.
packages/reflex-base/news/+config-reload-state-imports.bugfix.md Documents the user-visible duplicate-state reload fix.

Reviews (8): Last reviewed commit: "Track imports after failed package reloa..." | Re-trigger Greptile

@codspeed-hq

codspeed-hq Bot commented Sep 2, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 32 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing harsh21234i:fix/config-reload-state-imports (7669cec) with main (c49a85d)

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 3 files

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

Re-trigger cubic

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

@FarhanAliRaza FarhanAliRaza 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.

Tested with the appmod.py / rxconfig.py pair from #7028.

On main, get_config() followed by reload_config() in one RegistrationContext raises StateValueError. The new regression test fails there with that error and passes on this branch. ruff, pyright, and tests/units/test_config.py pass on the branch.

I also ran the same reload on a forked context, the shape AppHarness uses. That still raises the same StateValueError. I removed the _config_module_deps_root comparison and re-ran the config tests to check the root guard. They all pass without it.

See the inline comments for the requested changes.

# before probing: find_spec answers from sys.modules, so modules
# left behind by another project directory would fake the existence
# check below.
# Always reload rxconfig, but retain its dependencies when reloading

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.

Too long comments, explaining what is already can be seen in code. happens in many places.
we might want to clean up these.

"""
ctx = RegistrationContext.ensure_context()
config = _get_config()
config = _get_config(reload_dependencies=ctx._config is None)

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.

ctx._config is None is the wrong signal. RegistrationContext.fork() copies base_states but resets _config to None. So a forked context evicts and re-imports the state module into a context that already holds the class, and the shadow check fires again.

Repro with the appmod.py / rxconfig.py pair from #7028:

with RegistrationContext() as ctx:
    c.get_config()
    forked = ctx.fork()
    tok = RegistrationContext.set(forked)
    c.reload_config()
    # StateValueError: The substate class 'appmod____my_state' has been defined multiple times.

This is the path AppHarness takes (reflex/testing.py:286: fork, then reload_config()), so the harness still crashes on such a project. Please key the decision on whether the current context already holds the states those modules registered, not on whether it has a cached config. Add the fork case to the tests.

for dep in _config_module_deps:
sys.modules.pop(dep, None)
_config_module_deps.clear()
if reload_dependencies or _config_module_deps_root != project_root:

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.

The _config_module_deps_root != project_root branch is not exercised. With the comparison removed, all 118 tests in tests/units/test_config.py still pass. test_get_config_evicts_dependencies_from_another_project calls _get_config() with the default reload_dependencies=True, so it never reaches this check.

Either drop _config_module_deps_root or add a test that reloads in one context after the cwd moved to a second project. Note that in that scenario a same-named state module would still hit the shadow error, so the guard may not buy anything.

Comment thread tests/units/test_config.py Outdated
)

assert reflex_base.config._get_config(first_project).app_name == "first"
assert reflex_base.config._get_config(second_project).app_name == "second"

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.

Both calls use reload_dependencies=True, so this passes on main too (after the fixture is adjusted) and does not cover the new root check. To cover it, load first_project into a context, then call reload_config() from second_project in the same context.

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

Copy link
Copy Markdown
Contributor Author

Addressed the remaining review feedback in commit 4d8f57f.

  • Config reloads now restore only project-local modules that define states already registered in the active RegistrationContext. Ordinary imported config dependencies are evicted and re-imported, so edited values are not stale.
  • Added a same-project edited-dependency regression assertion.
  • Extracted the test module identifiers into named constants.
  • The forked-context, older-context, fresh-context, project-switch, and edited-dependency cases are covered.

Verification:

  • Focused reload tests: 4 passed.
  • Full tests/units/test_config.py: 117 passed; 2 unrelated deprecation tests could not run in my borrowed environment because granian is unavailable.
  • Ruff check, formatting, and git diff --check pass.

Please re-review the latest commit.

@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 3 files (changes from recent commits).

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

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_config.py Outdated
Comment on lines +966 to +967
for dep in sorted(package_modules, key=lambda name: name.count(".")):
importlib.reload(ctx._config_module_deps[dep])

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.

P1 State Package Gets Redefined

When a package defines a registered state in its __init__.py and also contains another state module, it belongs to both state_modules and package_modules. This unconditional reload re-executes the initializer and creates a new state class while the context still retains the original class. Module exports and the state registry can then refer to different class objects, defeating the state preservation this reload path is intended to provide.

Knowledge Base Used: Application lifecycle and configuration

@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 2 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="packages/reflex-base/src/reflex_base/config.py">

<violation number="1" location="packages/reflex-base/src/reflex_base/config.py:967">
P1: When a package's `__init__.py` defines a registered `rx.State`, this reload creates a replacement class while `RegistrationContext` retains the original. Skip reloading package modules that are themselves registered state modules.</violation>
</file>

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

Re-trigger cubic

sys.modules[dep] = module
_config_module_deps.add(dep)
for dep in sorted(package_modules, key=lambda name: name.count(".")):
importlib.reload(ctx._config_module_deps[dep])

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.

P1: When a package's __init__.py defines a registered rx.State, this reload creates a replacement class while RegistrationContext retains the original. Skip reloading package modules that are themselves registered state modules.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-base/src/reflex_base/config.py, line 967:

<comment>When a package's `__init__.py` defines a registered `rx.State`, this reload creates a replacement class while `RegistrationContext` retains the original. Skip reloading package modules that are themselves registered state modules.</comment>

<file context>
@@ -951,10 +951,20 @@ def _get_config(
                         sys.modules[dep] = module
                         _config_module_deps.add(dep)
+                for dep in sorted(package_modules, key=lambda name: name.count(".")):
+                    importlib.reload(ctx._config_module_deps[dep])
             # only import the module if it exists. If a module spec exists then
             # the module exists.
</file context>
Suggested change
importlib.reload(ctx._config_module_deps[dep])
if dep not in state_modules:
importlib.reload(ctx._config_module_deps[dep])

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

@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 2 files (changes from recent commits).

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

Re-trigger cubic

Comment thread packages/reflex-base/src/reflex_base/config.py Outdated
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.

Reloading a config whose rxconfig.py imports a state-defining module raises StateValueError

2 participants