fix(migration): support RESP3 key enumeration

[Kuang-xianxin]

Closes #713
Closes #714

On RESP3 connections, migration key enumeration reads byte-keyed FT.INFO maps as string-keyed dictionaries, treating failed or incomplete indexes as ready. Healthy indexes then hit a KeyError when the aggregate cursor's result map is sliced as a RESP2 row list.

The missing prefix also has a data-safety consequence: with RESP3 and decode_responses=False, if FT.AGGREGATE raises ResponseError, the SCAN fallback can use * instead of the source index's prefix. A field-rename migration can then rewrite a same-named field on unrelated applications' keys. This cross-prefix rewrite was reproduced on Redis 8.2.7 during review.

Changes:

• Normalize FT.INFO before checking readiness and extracting SCAN prefixes, keeping fallback enumeration scoped to the source index.
• Preserve a real zero for percent_indexed in enumeration, both executors' apply() guards, and the planner. Schema-only migrations of zero-progress indexes enumerate the stored keys and pass their count to validation; planning also emits the still-building warning.
• Share aggregate key extraction between sync and async executors, handling RESP2 rows and RESP3 result maps while retaining cursor pagination and cleanup.

Validation:

• Before the review follow-up, three new regression cases failed at d55117f: sync and async apply() omitted the source count, and the planner omitted its warning for percent_indexed=0.0. Nine controls passed. All 12 pass after the fix.
• After merging current main (2f8d3d3), all 162 related enumeration, executor, planner, validation/cluster-scan, pattern-escaping and batch-migration unit tests pass with redis-py 8.1.0 and separately with 6.3.0. The SCAN mocks bind redis-py's real scan_iter implementations to exercise the updated traversal.
• Full source mypy passes (120 files); repository-wide Black/isort and scoped codespell pass.
• These tests use mocked Redis responses. Run with pytest --confcutdir=tests/unit to exclude the root Docker startup fixture. The Redis-backed integration suite was not run locally because Docker is unavailable; the Redis 8.2.7 reproduction above is the reviewer's evidence.

Suggested release label: auto:patch.

---

Note

Medium Risk
Changes core migration enumeration and index-readiness logic; incorrect behavior could skip documents or fail mid-migration, though scope is limited to wire-format handling and existing SCAN fallbacks.

Overview
Fixes migration key enumeration on RESP3 (and mixed byte/string) Redis responses so drop/recreate migrations no longer mis-read index readiness or crash when paging FT.AGGREGATE cursors.

FT.INFO is normalized with convert_bytes before reading hash_indexing_failures, prefixes, and percent_indexed. A real 0 progress value is preserved (only None defaults to fully indexed), so unfinished indexes correctly trigger SCAN fallback instead of using the aggregate fast path.

Aggregate cursor pages share _extract_aggregate_keys, which reads document keys from RESP2 row lists or RESP3 results maps. Sync and async executors and the planner use the same percent_indexed rules when deciding enumeration and warnings.

Unit tests cover RESP2/RESP3 aggregate paging, SCAN fallback for failed/partial indexes, cursor cleanup, and schema-only migrations that must count keys when num_docs is misleading.

^{Reviewed by Cursor Bugbot for commit affa15a. Bugbot is set up for automated code reviews on this repo. Configure here.}

[vishal-bala]

Thanks for your contribution! This looks good, but it looks like there are a couple more spots where the fix should be applied and a couple details that could be clarified.

[vishal-bala]

The same expression survives at three sibling sites, all on the RESP3 path:

• redisvl/migration/executor.py:1116-1119
• redisvl/migration/async_executor.py:828-831
• redisvl/migration/planner.py:173-175

Those read stats_snapshot, which comes from SearchIndex.info() and is already normalised by convert_bytes, so the byte keys are not the problem there. The value type is. Measured on Redis 8.2.7 across all four protocol and decode_responses combinations, percent_indexed is the string '1' on RESP2 and the float 1.0 on RESP3, so a genuine zero is truthy on RESP2 and falsy on RESP3, where or 1.0 reads it as fully built.

executor.py:1119 feeds needs_exact_count, which gates needs_enumeration at :1120-1125; when false, the migration skips key enumeration altogether and validation falls back to the weaker num_docs comparison. That is the outer guard for the same condition this line now catches. planner.py:175 suppresses the "Source index is still building" warning in rvl migrate plan.

progress = snapshot.stats_snapshot.get("percent_indexed")
source_percent_indexed = float(progress) if progress is not None else 1.0

[Kuang-xianxin]

Fixed all three sites in affa15a. Both apply guards and the planner now default only when progress is None. The new schema-only apply tests use num_docs=0 with two stored keys and assert that expected_source_count=2 reaches validation. These sync/async cases and the planner's numeric-zero case failed before the fix; RESP2 string-zero and complete/null controls passed. After syncing main's scan_iter change, all 162 related tests pass on both redis-py 8.1.0 and 6.3.0.

[vishal-bala]

This is the most consequential change in the diff, and the description does not mention it. Worth adding, because it changes how urgent the fix looks to anyone triaging.

Measured pre-PR on RESP3 with decode_responses=False, _extract_prefixes_from_info returned [] against the byte-keyed FT.INFO. build_scan_match_patterns([]) returns ["*"] (utils.py:86-92), so _enumerate_with_scan enumerated the entire keyspace as the index's documents. _rename_field_in_hash at :640 does not filter the key list it is handed, so an unrelated key carrying a field of the same name gets rewritten:

otherapp:user:9  before: {'email': 'x@y.z', 'title': 'someone elses data'}
                 after : {'email': 'x@y.z', 'headline': 'someone elses data'}

Post-fix, prefix extraction returns the real prefix in all four combinations. Reaching the bad path needs FT.AGGREGATE to fail with a ResponseError first, which the _enumerate_with_aggregate docstring treats as routine cursor expiry rather than an exotic condition.

[Kuang-xianxin]

Expanded the description with the cross-prefix field-rename consequence and its trigger: RESP3 byte-keyed FT.INFO plus an aggregate ResponseError could turn the fallback into SCAN *. It now explicitly attributes the Redis 8.2.7 reproduction to your review. The existing sync/async fallback tests continue to assert the source-prefix match after the aggregate error.

[vishal-bala]

Measured on Redis 8.2.7, the leading element is not the row count: a two-row page came back as [1, [b'__key', b'a:0'], [b'__key', b'a:1']], while the same page reported total_results: 2 on RESP3. [1:] discards it either way, so only the comment is wrong.

[Kuang-xianxin]

Corrected the comment to call this leading metadata. The RESP2 page fixture now starts with 1 even for a two-row page, so pagination also verifies that extraction does not treat it as the row count.

[vishal-bala]

Measured on Redis 8.2.7, a freshly created index with num_docs: 0 reports percent_indexed of '1' on RESP2 and 1.0 on RESP3, with indexing: 0, so an empty index takes the aggregate fast path rather than the fallback. A percent_indexed of 0.0 means the background build has not started.

Your PR body already has the right word: "zero-progress". The id carries weight here because, with the executors reverted, empty-index is the only readiness value that fails at decode_responses=True, making it the one case that guards this change.

[Kuang-xianxin]

Renamed the case to zero-progress. The new apply regressions likewise distinguish zero background-build progress from a fully built index with no indexed documents.

[vishal-bala]

Dead: assert_not_called() at :131 makes this return_value unreachable by construction. Worth dropping the assignment and moving the comment down beside that assertion, where a reader meets it after the thing it justifies.

Two redundant axes while you are here. With the executors reverted, all three readiness values fail at decode_responses=False, all catching the same byte-key bug, so two of the three earn nothing there. And test_closing_enumeration_releases_cursor fails only at protocol=3, where cursor release reads result[1] identically in both response models, so that axis is already covered by test_enumerate_aggregate_cursor_pages.

[Kuang-xianxin]

Removed the unreachable return_value and moved its explanation beside assert_not_called(). Kept one byte-key readiness case plus the three decoded semantic cases, and restricted the cursor-close test to RESP3; the pagination test still covers both protocols. This reduces the enumeration file from 28 to 22 cases while preserving those distinct regressions.