Version 1.3.4 - #287
Conversation
…r, not a class variable
…nch on each step anymore
…d, we update base on the model settings, not the referred class
…s to determine hints
… dynamically created
…not refreshed on afetch when referred model has read update
…pe for find generic param
…ne for each branch
…n't dynamically created
…t need this check
…pes-in-rapyer] - changelog and version bump to 1.3.4 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…o feature/247-stop-using-dynamic-types-in-rapyer
- Delete unused extract_annotation from rapyer/utils/annotation.py - Delete its sole test file tests/unit/cascade/test_extract_annotation.py - field_with_flag (production superseder) and has_annotation untouched Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ecute sites - Add shared aexecute_pipeline_with_cascade_self_heal + aretry_fcall_after_missing_function to registry.py - ensure_pipeline/pipeline_with_execution route execute through the self-heal wrapper (lazy import, documented cycle) - aset_ttl routes bare execute through the self-heal wrapper - _apipeline replays FCALL on function-not-found, reusing aretry_fcall_after_missing_function - Rewrite config.py freeze-exempt comment; drop resolved issue #284 notes - Retry rewrites only the function-name slot; single retry then PersistentCascadeFunctionError - fakeredis EXPIRE branch and single-FCALL atomicity preserved Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…call in test helpers - Delete arun_fcall from registry.py (import + __all__ in scripts/__init__.py) - handle_missing_function and PersistentCascadeFunctionError retained (used by self-heal) - 3 integration _apply_cascade helpers call real_redis_client.fcall directly - Production self-heal now covers what arun_fcall did (Task 2) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…284, RED) - After FUNCTION FLUSH, aset_ttl(cascade=True) and refresh_ttl must reload the cascade function and still refresh the reachable subtree - Currently fails: redis-py's async pipeline masks the function-not-found message, so the helper's string match never fires inside a pipeline Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…not error text (GREEN) - redis-py's async pipeline annotate_exception (client.py:1585) overwrites exception.args with a non-f-string literal, destroying the "Function not found" message; every production FCALL runs inside a pipeline, so string-matching never fired there (only the deleted direct-client arun_fcall saw the real text) - Add acascade_function_missing (FUNCTION LIST scan) + _pipeline_has_fcall guard - Self-heal only fires on real Redis when the pipeline enqueued an FCALL and the cascade function is genuinely absent; other ResponseErrors re-raise unchanged - Wire the registry-based detection into both the shared wrapper and _apipeline Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…drop cascade self-heal helpers - context.py: ensure_pipeline + pipeline_with_execution use bare pipe.execute(), remove lazy registry imports - base.py: aset_ttl uses pipe.execute(); _apipeline reverts to NOSCRIPT-only path (no FCALL-missing detection/replay) - registry.py: delete six self-heal helpers; imports drop ResponseError, PersistentCascadeFunctionError, cascade.planner Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…te self-heal test, fix stale comments - errors/cascade.py + __init__.py: remove PersistentCascadeFunctionError and its __all__ entry - delete tests/integration/foreign_keys/test_cascade_self_heal.py - config.py: cascade_function_name freeze-exemption comment now references init_rapyer(), not the removed self-heal path Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Milestone: Configurable TTL Cascade
…rigin/develop' into codex/fix-init-rapyer-connection-rebind # Conflicts: # rapyer/init.py
…onnection-rebind Fix init_rapyer connection rebinding order
There was a problem hiding this comment.
Semgrep OSS found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
| SF_SAVE_FILENAME = "save.lua" | ||
| SF_LOAD_FILENAME = "load.lua" | ||
| CASCADE_LIB_TOKEN = "RAPYER_CASCADE_LIB" | ||
| CASCADE_FN_TOKEN = "RAPYER_CASCADE_FN" |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds configurable TTL cascading through Redis Functions, including graph planning, depth controls, dangling-key results, initialization validation, and special-field refreshes. Refactors generic Redis type handling and relational resolution, expands Redis and fakeredis tests, and updates CI workflows, Redis support, documentation, and release metadata. ChangesTTL cascade contracts and initialization
Generic type and serialization plumbing
Repository automation and release configuration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Application
participant AtomicRedisModel
participant CascadePlanner
participant RedisFunction
participant Redis
Application->>AtomicRedisModel: init_rapyer(cascade_ttl)
AtomicRedisModel->>CascadePlanner: build and validate plan
CascadePlanner-->>AtomicRedisModel: plan JSON and function names
AtomicRedisModel->>Redis: register cascade function
Application->>AtomicRedisModel: aset_ttl(cascade=True)
AtomicRedisModel->>RedisFunction: FCALL cascade_apply
RedisFunction->>Redis: JSON.GET references and EXPIRE keys
Redis-->>RedisFunction: dangling child and special counts
RedisFunction-->>AtomicRedisModel: CascadeResult payload
AtomicRedisModel-->>Application: CascadeResult
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
| raise | ||
| try: | ||
| await model.adelete_index() | ||
| except ResponseError: |
Coverage reportTotal coverage: 99% Full report |
Merging this PR will regress 0 benchmarks
Performance Changes
Tip Curious why this is faster? Comment Comparing |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
tests/unit/cascade/conftest.py (1)
74-125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated setup/teardown logic between the two fixtures.
setup_fake_redis_for_cascade_modelsandsetup_fake_redis_for_cascade_applydiffer only by theregister_scriptscall, yet both fully repeat the snapshot/wire/restore loop. Extracting the shared body (e.g. a helper context manager, or having the async fixture wrap the sync one) would avoid the two copies drifting if the snapshot/restore fields ever change.🤖 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/unit/cascade/conftest.py` around lines 74 - 125, Extract the duplicated model snapshot, fakeredis wiring, relational-target resolution, and teardown logic from setup_fake_redis_for_cascade_models and setup_fake_redis_for_cascade_apply into one shared helper or fixture. Keep register_scripts(fake_redis_client, is_fakeredis=True) only in the cascade-apply setup path, while ensuring both fixtures restore all original Meta fields after yielding.tests/integration/foreign_keys/test_cascade_depth_and_gate.py (1)
27-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame
_apply_cascadeFCALL helper copy-pasted across three integration test files. All three define an identical async helper that callsreal_redis_client.fcall(type(root).Meta.cascade_function_name, 1, root.key, type(root).__name__, SPECIAL_FIELD_KEY_PREFIX, type(root).Meta.ttl, 1 if cascade else 0). If the FCALL argument order or signature ever changes, all three copies must be updated in lockstep or tests silently diverge.
tests/integration/foreign_keys/test_cascade_depth_and_gate.py#L27-L36: move this helper into a shared conftest/fixture module and import it here.tests/integration/foreign_keys/test_cascade_graph_shapes.py#L16-L25: import the shared helper instead of redefining it.tests/integration/foreign_keys/test_cascade_ttl_apply.py#L21-L30: import the shared helper instead of redefining it.🤖 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/integration/foreign_keys/test_cascade_depth_and_gate.py` around lines 27 - 36, Move the duplicated async _apply_cascade helper into a shared conftest or fixture module, preserving its existing FCALL argument order and behavior. In tests/integration/foreign_keys/test_cascade_depth_and_gate.py#L27-L36, define and expose the shared helper, then import it in tests/integration/foreign_keys/test_cascade_graph_shapes.py#L16-L25 and tests/integration/foreign_keys/test_cascade_ttl_apply.py#L21-L30 while removing their local definitions.tests/models/cascade_types.py (1)
317-345: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCascade fixture model list duplicated verbatim in two files.
ALL_CASCADE_MODELSandCASCADE_PLANNER_MODELScontain the exact same 27 classes in the exact same order, maintained independently.
tests/models/cascade_types.py#L317-L345: keepALL_CASCADE_MODELSas the single source of truth.tests/unit/cascade/conftest.py#L36-L64: import and reuseALL_CASCADE_MODELSfromtests/models/cascade_types.pyinstead of redefiningCASCADE_PLANNER_MODELS.🤖 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/models/cascade_types.py` around lines 317 - 345, Keep ALL_CASCADE_MODELS in tests/models/cascade_types.py as the single source of truth without changing its contents. In tests/unit/cascade/conftest.py, remove the duplicated CASCADE_PLANNER_MODELS definition and import/reuse ALL_CASCADE_MODELS from tests/models/cascade_types.py; update any references if needed so planner fixtures use the shared list.rapyer/types/base.py (1)
26-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
wrapped_python_typehas no default, andRedisBytesis the one subclass that never sets it.The base
ClassVar[type]declaration has no fallback, so it's only safe as long as every subclass either sets it or (likeRedisBytes) fully overrides__get_pydantic_core_schema__to avoid touching it. That's fragile for future maintenance — any generic introspection ofwrapped_python_typeacross redis types would blow up onRedisBytes.
rapyer/types/base.py#L26-L30: givewrapped_python_typea safe default (or an explicit "not implemented" property) onBaseRedisTypeso subclasses that forget to set it fail predictably rather than only by accident of which methods they happen to override.rapyer/types/byte.py#L10-L36: declarewrapped_python_type: ClassVar[type] = bytesonRedisBytesfor consistency withRedisStr/RedisDatetime/RedisList, even though the custom schema override doesn't currently need it.🤖 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 `@rapyer/types/base.py` around lines 26 - 30, Provide a safe default for wrapped_python_type on BaseRedisType, then explicitly set wrapped_python_type to bytes on RedisBytes for consistency with the other Redis type subclasses. Update rapyer/types/base.py lines 26-30 and rapyer/types/byte.py lines 10-36; no other behavior needs to change.rapyer/base.py (1)
866-874: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
_ttl_keys()helper. Nothing in the repo calls it anymore, so it can be dropped unless there’s an external caller.🤖 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 `@rapyer/base.py` around lines 866 - 874, Remove the unused _ttl_keys method from the model class, including its docstring and list construction; do not alter _iter_special_fields or other TTL-related behavior.rapyer/types/convert.py (1)
95-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix typo in method name.
The method name
covert_generic_typehas a typo (missing 'r'). Consider renaming it toconvert_generic_type(and updating any callers, e.g., inrapyer.utils.annotation) to maintain naming consistency withconvert_flat_type.♻️ Proposed refactor
- def covert_generic_type( - self, type_to_covert: type, generic_values: tuple[type] + def convert_generic_type( + self, type_to_convert: type, generic_values: tuple[type] ) -> type:🤖 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 `@rapyer/types/convert.py` around lines 95 - 97, Rename the TypeConverter method covert_generic_type to convert_generic_type, and update every caller such as rapyer.utils.annotation to use the corrected name while preserving the existing behavior and consistency with convert_flat_type.rapyer/init.py (1)
76-81: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFilter
ResponseErrorto avoid silently swallowing unexpected initialization failures.When
override_old_idxisFalse, catching and indiscriminately swallowingResponseErrorduring index creation successfully suppresses the expected "Index already exists" exception. However, it will also silently suppress actual critical failures (e.g., RediSearch module missing, schema syntax errors, invalid configurations), leaving the application running without an index.Consider checking the error message so you only suppress the benign index-existence error.
🛠️ Proposed fix to narrow the suppression
- try: - await model.acreate_index() - except ResponseError: - if override_old_idx: - raise + try: + await model.acreate_index() + except ResponseError as e: + if override_old_idx or "Index already exists" not in str(e): + raise🤖 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 `@rapyer/init.py` around lines 76 - 81, Update the ResponseError handling around model.acreate_index() in initialization to suppress only the known “index already exists” condition when override_old_idx is false. Re-raise all other ResponseError instances, including module, schema, and configuration failures; preserve the current re-raise behavior when override_old_idx is true.
🤖 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 `@rapyer/types/lst.py`:
- Around line 36-39: Update the build_typed_original classmethod to handle empty
source_args before accessing source_args[0]. For unparameterized RedisList/list
annotations, return the established Any fallback or explicitly reject them;
preserve the existing parameterized behavior.
In `@tests/unit/cascade/test_init_rapyer_cascade_ttl.py`:
- Around line 56-58: Wrap the `init_rapyer(mock_redis_client)` call in the
test’s `REDIS_MODELS` patch context, patching `rapyer.init.REDIS_MODELS` with
the existing `cascade_models` fixture. Keep the test’s current arrange and
assertion behavior unchanged while ensuring initialization processes only the
fixture models.
---
Nitpick comments:
In `@rapyer/base.py`:
- Around line 866-874: Remove the unused _ttl_keys method from the model class,
including its docstring and list construction; do not alter _iter_special_fields
or other TTL-related behavior.
In `@rapyer/init.py`:
- Around line 76-81: Update the ResponseError handling around
model.acreate_index() in initialization to suppress only the known “index
already exists” condition when override_old_idx is false. Re-raise all other
ResponseError instances, including module, schema, and configuration failures;
preserve the current re-raise behavior when override_old_idx is true.
In `@rapyer/types/base.py`:
- Around line 26-30: Provide a safe default for wrapped_python_type on
BaseRedisType, then explicitly set wrapped_python_type to bytes on RedisBytes
for consistency with the other Redis type subclasses. Update
rapyer/types/base.py lines 26-30 and rapyer/types/byte.py lines 10-36; no other
behavior needs to change.
In `@rapyer/types/convert.py`:
- Around line 95-97: Rename the TypeConverter method covert_generic_type to
convert_generic_type, and update every caller such as rapyer.utils.annotation to
use the corrected name while preserving the existing behavior and consistency
with convert_flat_type.
In `@tests/integration/foreign_keys/test_cascade_depth_and_gate.py`:
- Around line 27-36: Move the duplicated async _apply_cascade helper into a
shared conftest or fixture module, preserving its existing FCALL argument order
and behavior. In
tests/integration/foreign_keys/test_cascade_depth_and_gate.py#L27-L36, define
and expose the shared helper, then import it in
tests/integration/foreign_keys/test_cascade_graph_shapes.py#L16-L25 and
tests/integration/foreign_keys/test_cascade_ttl_apply.py#L21-L30 while removing
their local definitions.
In `@tests/models/cascade_types.py`:
- Around line 317-345: Keep ALL_CASCADE_MODELS in tests/models/cascade_types.py
as the single source of truth without changing its contents. In
tests/unit/cascade/conftest.py, remove the duplicated CASCADE_PLANNER_MODELS
definition and import/reuse ALL_CASCADE_MODELS from
tests/models/cascade_types.py; update any references if needed so planner
fixtures use the shared list.
In `@tests/unit/cascade/conftest.py`:
- Around line 74-125: Extract the duplicated model snapshot, fakeredis wiring,
relational-target resolution, and teardown logic from
setup_fake_redis_for_cascade_models and setup_fake_redis_for_cascade_apply into
one shared helper or fixture. Keep register_scripts(fake_redis_client,
is_fakeredis=True) only in the cascade-apply setup path, while ensuring both
fixtures restore all original Meta fields after yielding.
🪄 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
Run ID: 46f99462-d0dd-47ee-a83f-b009971d80c3
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!**/*.lock,!**/uv.lock
📒 Files selected for processing (96)
.coderabbit.yaml.github/dependabot.yml.github/redis-matrix.json.github/workflows/bandit.yml.github/workflows/ci.yml.github/workflows/codeql.yml.github/workflows/coverage.yml.github/workflows/docs.yml.github/workflows/pm.yml.github/workflows/publish.yml.github/workflows/release.yml.github/workflows/security.yml.github/workflows/semgrep.yml.github/workflows/speed.ymlCHANGELOG.mdbenchmarks/conftest.pydocs/documentation/initialization.mddocs/documentation/special-fields/foreign-keys.mddocs/documentation/special-fields/priority-queue.mddocs/documentation/special-fields/ttl-cascade.mdmkdocs.ymlpyproject.tomlrapyer/__init__.pyrapyer/base.pyrapyer/cascade/__init__.pyrapyer/cascade/planner.pyrapyer/cascade/spec.pyrapyer/cascade/ttl.pyrapyer/config.pyrapyer/errors/__init__.pyrapyer/errors/cascade.pyrapyer/init.pyrapyer/result.pyrapyer/scripts/__init__.pyrapyer/scripts/constants.pyrapyer/scripts/loader.pyrapyer/scripts/lua/cascade/__init__.pyrapyer/scripts/lua/cascade/library.luarapyer/scripts/registry.pyrapyer/types/base.pyrapyer/types/byte.pyrapyer/types/convert.pyrapyer/types/datetime.pyrapyer/types/dct.pyrapyer/types/float.pyrapyer/types/foreign_key.pyrapyer/types/generic.pyrapyer/types/integer.pyrapyer/types/lst.pyrapyer/types/priority_queue.pyrapyer/types/redis_set.pyrapyer/types/relational.pyrapyer/types/string.pyrapyer/utils/annotation.pyrapyer/utils/pythonic.pytests/action_groups.pytests/conftest.pytests/integration/actions/redis_types/test_foreign_key.pytests/integration/actions/ttl.pytests/integration/conftest.pytests/integration/foreign_keys/conftest.pytests/integration/foreign_keys/test_cascade_action_boundary.pytests/integration/foreign_keys/test_cascade_concurrent_mutation.pytests/integration/foreign_keys/test_cascade_depth_and_gate.pytests/integration/foreign_keys/test_cascade_graph_shapes.pytests/integration/foreign_keys/test_cascade_ttl_apply.pytests/integration/foreign_keys/test_foreign_key.pytests/integration/functioninality/test_rapyer_aget.pytests/integration/lst/test_redis_list_remove_range.pytests/integration/pipeline/test_pipeline_noscript_recovery.pytests/models/cascade_types.pytests/models/collection_types.pytests/models/foreign_key_types.pytests/models/registry.pytests/models/special_types.pytests/unit/cascade/__init__.pytests/unit/cascade/conftest.pytests/unit/cascade/test_aset_ttl_cascade_flag.pytests/unit/cascade/test_cascade_action_boundary.pytests/unit/cascade/test_cascade_apply_lua_syntax.pytests/unit/cascade/test_cascade_classification.pytests/unit/cascade/test_cascade_plan_injection.pytests/unit/cascade/test_cascade_plan_table.pytests/unit/cascade/test_cascade_ttl_config.pytests/unit/cascade/test_cascade_ttl_required_validation.pytests/unit/cascade/test_init_rapyer_cascade_ttl.pytests/unit/cascade/test_meta_ttl_freeze.pytests/unit/cascade/test_refresh_ttl_cascade_branch.pytests/unit/conftest.pytests/unit/models/test_model_dump.pytests/unit/test_base_functions.pytests/unit/test_init_rapyer.pytests/unit/test_refresh_ttl_if_needed.pytests/unit/types/test_foreign_key.pytests/unit/types/test_special_types.pytox.ini
| # Act | ||
| await init_rapyer(mock_redis_client) | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add the missing REDIS_MODELS patch context to isolate the test.
This test requests the cascade_models fixture but currently omits the with patch("rapyer.init.REDIS_MODELS", cascade_models): block that is used in the adjacent test. Without it, init_rapyer() will process the entire global REDIS_MODELS registry, which couples the test's success to the stability of the global registry and introduces unnecessary processing overhead.
🛠️ Proposed fix
- # Act
- await init_rapyer(mock_redis_client)
+ # Act
+ with patch("rapyer.init.REDIS_MODELS", cascade_models):
+ await init_rapyer(mock_redis_client)📝 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.
| # Act | |
| await init_rapyer(mock_redis_client) | |
| # Act | |
| with patch("rapyer.init.REDIS_MODELS", cascade_models): | |
| await init_rapyer(mock_redis_client) |
🤖 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/unit/cascade/test_init_rapyer_cascade_ttl.py` around lines 56 - 58,
Wrap the `init_rapyer(mock_redis_client)` call in the test’s `REDIS_MODELS`
patch context, patching `rapyer.init.REDIS_MODELS` with the existing
`cascade_models` fixture. Keep the test’s current arrange and assertion behavior
unchanged while ensuring initialization processes only the fixture models.
…or FK graphs Benchmark aset_ttl(cascade=True) across three FK shapes: a root with two direct FKs, a 10-layer FK chain, and a root holding a list of 10 FKs. Chain uses a per-field CascadeTTL marker (not Meta.cascade_ttl, which init_rapyer resets to None when no cascade arg is passed) so the whole chain is walked. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary by CodeRabbit
CascadeResultwith dangling counts.CascadeTTLand result support.RedisPriorityQueueexclude=Truerequirements.