feat(cache): make REFLEXIO_CACHE_MAX_SIZE env-tunable (Workstream C phase 1) - #346
Merged
Merged
Conversation
📝 WalkthroughWalkthroughChangesCache capacity configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/server/cache/test_reflexio_cache.py`:
- Around line 482-504: Replace importlib.reload usage in
test_cache_max_size_env_override and test_cache_max_size_defaults_to_100 with
isolated module imports by temporarily removing
reflexio.server.cache.reflexio_cache from sys.modules, importing it fresh under
each environment setup, and restoring the original module entry afterward.
Remove the manual try/finally environment cleanup and rely on monkeypatch to
undo environment changes, while preserving the existing assertions.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 457dd930-9b3c-4102-9615-81b61e519ac5
📒 Files selected for processing (2)
reflexio/server/cache/reflexio_cache.pytests/server/cache/test_reflexio_cache.py
Comment on lines
+482
to
+504
| def test_cache_max_size_env_override(monkeypatch: pytest.MonkeyPatch): | ||
| """REFLEXIO_CACHE_MAX_SIZE is read from the environment at import time.""" | ||
| import reflexio.server.cache.reflexio_cache as cache_mod | ||
|
|
||
| monkeypatch.setenv("REFLEXIO_CACHE_MAX_SIZE", "400") | ||
| try: | ||
| cache_mod = importlib.reload(cache_mod) | ||
| assert cache_mod.REFLEXIO_CACHE_MAX_SIZE == 400 | ||
| assert cache_mod.get_cache_stats()["max_size"] == 400 | ||
| finally: | ||
| # Restore the module-global cache to its default sizing so the | ||
| # resized cache doesn't leak into other tests. | ||
| monkeypatch.delenv("REFLEXIO_CACHE_MAX_SIZE", raising=False) | ||
| importlib.reload(cache_mod) | ||
|
|
||
|
|
||
| def test_cache_max_size_defaults_to_100(monkeypatch: pytest.MonkeyPatch): | ||
| """Without the env var set, the max size falls back to 100.""" | ||
| import reflexio.server.cache.reflexio_cache as cache_mod | ||
|
|
||
| monkeypatch.delenv("REFLEXIO_CACHE_MAX_SIZE", raising=False) | ||
| cache_mod = importlib.reload(cache_mod) | ||
| assert cache_mod.REFLEXIO_CACHE_MAX_SIZE == 100 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Avoid using importlib.reload to prevent test pollution.
Using importlib.reload(cache_mod) mutates the module's dictionary in-place. If any functions in this module are wrapped by decorators that capture state at import time (such as @cached(cache=_reflexio_cache)), those decorators will hold a reference to the old cache, while dynamic lookups (like get_cache_stats()) will reference the newly created cache. This desynchronization causes random test failures depending on the execution order (e.g., via pytest-randomly), as subsequent tests might populate the old cache but assert on the new one.
Instead of reloading the module, temporarily remove it from sys.modules to force a clean, side-effect-free import that creates a completely new module instance. This fully isolates the test and leaves the original module (and any existing references) fully intact. Additionally, pytest's monkeypatch automatically undoes env var mutations at the end of the test, meaning the manual try...finally block can be safely eliminated.
🛠️ Proposed fix to isolate the module import
-def test_cache_max_size_env_override(monkeypatch: pytest.MonkeyPatch):
- """REFLEXIO_CACHE_MAX_SIZE is read from the environment at import time."""
- import reflexio.server.cache.reflexio_cache as cache_mod
-
- monkeypatch.setenv("REFLEXIO_CACHE_MAX_SIZE", "400")
- try:
- cache_mod = importlib.reload(cache_mod)
- assert cache_mod.REFLEXIO_CACHE_MAX_SIZE == 400
- assert cache_mod.get_cache_stats()["max_size"] == 400
- finally:
- # Restore the module-global cache to its default sizing so the
- # resized cache doesn't leak into other tests.
- monkeypatch.delenv("REFLEXIO_CACHE_MAX_SIZE", raising=False)
- importlib.reload(cache_mod)
-
-
-def test_cache_max_size_defaults_to_100(monkeypatch: pytest.MonkeyPatch):
- """Without the env var set, the max size falls back to 100."""
- import reflexio.server.cache.reflexio_cache as cache_mod
-
- monkeypatch.delenv("REFLEXIO_CACHE_MAX_SIZE", raising=False)
- cache_mod = importlib.reload(cache_mod)
- assert cache_mod.REFLEXIO_CACHE_MAX_SIZE == 100
+import sys
+from contextlib import contextmanager
+
+@contextmanager
+def _isolated_import(module_name: str):
+ """Temporarily remove a module from sys.modules to force a clean, side-effect-free import."""
+ old_mod = sys.modules.pop(module_name, None)
+ try:
+ yield importlib.import_module(module_name)
+ finally:
+ if old_mod is not None:
+ sys.modules[module_name] = old_mod
+ else:
+ sys.modules.pop(module_name, None)
+
+
+def test_cache_max_size_env_override(monkeypatch: pytest.MonkeyPatch):
+ """REFLEXIO_CACHE_MAX_SIZE is read from the environment at import time."""
+ monkeypatch.setenv("REFLEXIO_CACHE_MAX_SIZE", "400")
+ with _isolated_import("reflexio.server.cache.reflexio_cache") as cache_mod:
+ assert cache_mod.REFLEXIO_CACHE_MAX_SIZE == 400
+ assert cache_mod.get_cache_stats()["max_size"] == 400
+
+
+def test_cache_max_size_defaults_to_100(monkeypatch: pytest.MonkeyPatch):
+ """Without the env var set, the max size falls back to 100."""
+ monkeypatch.delenv("REFLEXIO_CACHE_MAX_SIZE", raising=False)
+ with _isolated_import("reflexio.server.cache.reflexio_cache") as cache_mod:
+ assert cache_mod.REFLEXIO_CACHE_MAX_SIZE == 100📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def test_cache_max_size_env_override(monkeypatch: pytest.MonkeyPatch): | |
| """REFLEXIO_CACHE_MAX_SIZE is read from the environment at import time.""" | |
| import reflexio.server.cache.reflexio_cache as cache_mod | |
| monkeypatch.setenv("REFLEXIO_CACHE_MAX_SIZE", "400") | |
| try: | |
| cache_mod = importlib.reload(cache_mod) | |
| assert cache_mod.REFLEXIO_CACHE_MAX_SIZE == 400 | |
| assert cache_mod.get_cache_stats()["max_size"] == 400 | |
| finally: | |
| # Restore the module-global cache to its default sizing so the | |
| # resized cache doesn't leak into other tests. | |
| monkeypatch.delenv("REFLEXIO_CACHE_MAX_SIZE", raising=False) | |
| importlib.reload(cache_mod) | |
| def test_cache_max_size_defaults_to_100(monkeypatch: pytest.MonkeyPatch): | |
| """Without the env var set, the max size falls back to 100.""" | |
| import reflexio.server.cache.reflexio_cache as cache_mod | |
| monkeypatch.delenv("REFLEXIO_CACHE_MAX_SIZE", raising=False) | |
| cache_mod = importlib.reload(cache_mod) | |
| assert cache_mod.REFLEXIO_CACHE_MAX_SIZE == 100 | |
| import sys | |
| from contextlib import contextmanager | |
| `@contextmanager` | |
| def _isolated_import(module_name: str): | |
| """Temporarily remove a module from sys.modules to force a clean, side-effect-free import.""" | |
| old_mod = sys.modules.pop(module_name, None) | |
| try: | |
| yield importlib.import_module(module_name) | |
| finally: | |
| if old_mod is not None: | |
| sys.modules[module_name] = old_mod | |
| else: | |
| sys.modules.pop(module_name, None) | |
| def test_cache_max_size_env_override(monkeypatch: pytest.MonkeyPatch): | |
| """REFLEXIO_CACHE_MAX_SIZE is read from the environment at import time.""" | |
| monkeypatch.setenv("REFLEXIO_CACHE_MAX_SIZE", "400") | |
| with _isolated_import("reflexio.server.cache.reflexio_cache") as cache_mod: | |
| assert cache_mod.REFLEXIO_CACHE_MAX_SIZE == 400 | |
| assert cache_mod.get_cache_stats()["max_size"] == 400 | |
| def test_cache_max_size_defaults_to_100(monkeypatch: pytest.MonkeyPatch): | |
| """Without the env var set, the max size falls back to 100.""" | |
| monkeypatch.delenv("REFLEXIO_CACHE_MAX_SIZE", raising=False) | |
| with _isolated_import("reflexio.server.cache.reflexio_cache") as cache_mod: | |
| assert cache_mod.REFLEXIO_CACHE_MAX_SIZE == 100 |
🤖 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 `@tests/server/cache/test_reflexio_cache.py` around lines 482 - 504, Replace
importlib.reload usage in test_cache_max_size_env_override and
test_cache_max_size_defaults_to_100 with isolated module imports by temporarily
removing reflexio.server.cache.reflexio_cache from sys.modules, importing it
fresh under each environment setup, and restoring the original module entry
afterward. Remove the manual try/finally environment cleanup and rely on
monkeypatch to undo environment changes, while preserving the existing
assertions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Make the Reflexio instance-cache max size operator-tunable via a new env var
REFLEXIO_CACHE_MAX_SIZE(default 100, unchanged behavior until set).Why
Part of the Scalability-Readiness Workstream C (instance-aware limits & caches). The
in-process instance cache holds up to 100 warm per-org
Reflexioinstances; at ~300concurrently-active orgs per task the LRU thrashes and every request cold-rebuilds. This lets
an operator size the cache to the active-org count. No behavior change until the var is set.
Change
reflexio_cache.py: read the size via the existingpositive_int_env("REFLEXIO_CACHE_MAX_SIZE", 100, logger)(import-light helper; falls back to 100 on unset/blank/invalid/non-positive).importlib.reload, since the value is read once at import) + default-when-unset; teardown restores the default cache.Design:
docs/superpowers/specs/2026-07-14-workstream-c-instance-aware-limits-caches-design.md§3 (in the enterprise repo).Summary by CodeRabbit
New Features
REFLEXIO_CACHE_MAX_SIZEenvironment setting.Tests