feat: add iter()/aiter() for lazy filter-based key iteration

[Aryan-Pardeshi]

Fixes #489

Adds iter_keys() and aiter_keys() for lazy, filter-based iteration over the keys in an index.

Neither existing API covers this: paginate() yields full document records rather than keys, and scan_by_pattern() works on raw Redis key patterns and materialises a list. For index maintenance over a large index you want keys only, streamed, and selectable by filter expression.

for key in index.iter_keys(Tag("category") == "A"):
    ...

Both take an optional FilterExpression (defaulting to match-all) and a batch_size defaulting to DEFAULT_BULK_BATCH_SIZE, page through _iter_keys_by_filter with FT.AGGREGATE ... WITHCURSOR, and yield keys one at a time without ever building the full list. The async version mirrors the sync one exactly.

tests/integration/test_index_iteration.py covers full iteration, filtered iteration, laziness (the first key arrives without draining the index), and a batch_size smaller than the document count so the paging loop is actually exercised rather than short-circuiting on a single batch. 7 passed against Redis in Docker.

Two things I want to flag rather than have you find:

iter_keys does not shadow the builtin as a method name. It returns document keys (not full records) with an optional FilterExpression and configurable batch_size, defaulting to match-all when the filter is omitted.

I did not add __iter__/__aiter__ dunders. Making a SearchIndex directly iterable would mean list(index) silently issues a full paged scan against Redis, which felt like a surprising thing to attach to a plain for loop. Easy to add if you want it.

Overview
Adds iter_keys() on SearchIndex and aiter_keys() on AsyncSearchIndex so callers can walk document Redis keys (not full records) with an optional FilterExpression and configurable batch_size, defaulting to match-all when the filter is omitted.

Both methods are thin wrappers around the existing _iter_keys_by_filter path (FT.AGGREGATE + WITHCURSOR), so iteration avoids MAXSEARCHRESULTS caps that affect FT.SEARCH + LIMIT, with the same de-duplication and memory characteristics documented on that helper.

Integration tests in test_index_iteration.py cover full and filtered scans, lazy consumption, and paging when batch_size is smaller than the index size for sync and async.

Notes:

• A batch can come back smaller than batch_size (an all-repeat cursor page yields nothing), and a full drain is no proof every match was seen, since the cursor is a position in an ascending walk of internal document ids, not a snapshot. Callers that need completeness should reconcile against a CountQuery rather than trusting a drained cursor.
• The server-side cursor's idle timeout only resets when a page is read, so a caller doing per-key work (embedding, network) can have its cursor reaped after keys have already been yielded, which fails mid-stream rather than up front. The cursor is released on normal exit, early break, and error; the abandon-and-never-close case cannot be fixed from in here, so callers who break out early should wrap the iterator in contextlib.aclosing().
• Do not concatenate untrusted input into filter_expression — it is rendered into the FT.AGGREGATE request verbatim, the same constraint drop_by_filter and update_by_filter carry.
• None, "", whitespace, and a default FilterExpression are all treated as match-all, consistent with drop_by_filter and update_by_filter.

Closes #489

---

Note

Low Risk
Additive public API on top of an existing internal cursor helper; read-only iteration with documented operational caveats and integration test coverage.

Overview
Adds iter_keys() on SearchIndex and aiter_keys() on AsyncSearchIndex so callers can lazily walk document Redis keys (not full records) with an optional FilterExpression and configurable batch_size, defaulting to match-all via the same _is_match_all_filter rules as other bulk filter APIs.

Both methods validate batch_size, normalize match-all filters to *, and stream keys one at a time through the existing _iter_keys_by_filter path (FT.AGGREGATE + WITHCURSOR), avoiding MAXSEARCHRESULTS limits from FT.SEARCH pagination. The async variant uses contextlib.aclosing on each batch when flattening pages. Docstrings document cursor semantics, idle timeout risks, and filter-injection caveats.

New integration tests in test_index_iteration.py cover full/filtered iteration, laziness, small batch_size paging, async parity, and invalid batch_size errors.

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

[cursor]

Stale Bugbot comment from a previous run.

[Aryan-Pardeshi]

Good catch — pushed a fix. iter()/aiter() now delegate to _iter_keys_by_filter(), which pages with FT.AGGREGATE ... WITHCURSOR instead of FT.SEARCH + LIMIT, so this is no longer subject to MAXSEARCHRESULTS. Tests still pass (7/7), full unit suite green (1292 passed).

[vishal-bala]

Thanks for this. The mechanics are right and the docstrings are candid about the trade-offs, which made it straightforward to review. I ran it against Redis 8.2.7 and iteration returned every key, filtered correctly with both a Tag builder and a raw "@category:{A}" string, and paged properly at batch_size=2. Five inline comments, plus two things here.

Please rename both methods to iter_keys, on SearchIndex and AsyncSearchIndex alike. Every other paired method in the file keeps one name across the two classes, paginate included, so porting a loop by swapping the class currently raises AttributeError. iter_keys also says what gets yielded and reads well next to drop_keys. Leaving out the __iter__/__aiter__ dunders was the right call for the reason you gave.

The description still describes your first commit, with _query, return_fields=["id"], and keys yielded "without ever building the full list". Both changed in the FT.AGGREGATE rewrite. Please update it.

[vishal-bala]

This assertion can't fail. Keys are strings, so is not None holds for anything the generator yields, and an empty index would surface as an uncaught StopIteration rather than a failed assertion.

I checked by swapping in an iter() that drains the whole index into a list before yielding anything. The test still passed, so it can't tell a streaming implementation from an eager one, which is the one property it's named for.

What would work is counting FT.AGGREGATE round trips before the first key arrives. tests/unit/test_bulk_cursor_dedup.py already has a _ReplayCursor harness that fakes client.ft(name), so this can be a hermetic unit test with no server and no fixture. While you're there, the async method has no laziness test at all.

[vishal-bala]

The server-side cursor can outlive this generator. Measured on Redis 8.2.7: break out of async for key in index.aiter() early, then await index.disconnect(), and FT.INFO ... cursor_stats shows index_total climbing 1, 2, 3 across three asyncio.run lifecycles. The cursor is never released. The event loop's async-generator finalisation tries to send FT.CURSOR DEL after the client has gone, and the except RedisError: pass in the helper swallows the failure. Each leaked cursor is held for MAXIDLE 300 seconds, against a per-shard capacity of 128.

Two parts. Wrapping the inner generator makes an explicit close deterministic:

async with contextlib.aclosing(
    self._iter_keys_by_filter(filter_expr, batch_size)
) as batches:
    async for batch in batches:
        for key in batch:
            yield key

I measured the cursor released the instant aclose() returns with that in place, versus still open without it. The abandon-and-never-close case can't be fixed from in here, so the docstring should tell callers who break out early to wrap the iterator in contextlib.aclosing().

The sync path needs nothing. I watched index_total drop back to 0 on close(), on del plus a collection, and on break.

[vishal-bala]

This wrapper is a no-op, and bypassing the existing helper introduces a divergence. str(FilterExpression("*")) is just "*", and _iter_keys_by_filter already accepts a plain str.

_is_match_all_filter at index.py:236 is what drop_by_filter and update_by_filter use for this normalisation, and it treats "" and whitespace as match-all as well as None. Measured: iter("") and iter(" ") return zero keys here, while drop_by_filter("") matches the whole index. So a filter that arrives empty from config means nothing to one method and everything to its neighbour. iter(FilterExpression()) also raises the bare ValueError("Improperly initialized FilterExpression") that the helper exists to absorb.

Routing through it fixes all three cases and deletes these three lines.

[vishal-bala]

batch_size wants the same guards paginate has fifteen lines up: TypeError for a non-int, ValueError for anything below 1.

Measured on Redis 8.2.7. batch_size=0 silently returns every key, because redis-py drops a falsy COUNT and the server picks its own page size, which makes "Defaults to 500" in the docstring untrue. batch_size="5" sails straight through. batch_size=-1 surfaces the raw server text Bad arguments for COUNT: Value is outside acceptable bounds inside a RedisSearchError.

Matching paginate is enough. Worth knowing that neither will raise at call time, since validation inside a generator function doesn't run until the first next(). If you'd rather it fail eagerly, the checks have to live in a non-generator wrapper that returns the generator. Your call.

[vishal-bala]

These caveats point at something the published docs don't contain. docs/api/searchindex.rst uses autoclass ... :members: with no :private-members:, so iter appears on docs.redisvl.com but _iter_keys_by_filter doesn't. The :meth: reference is dangling and the memory and completeness caveats are invisible to exactly the reader who needs them. The async method is a longer chain still, because the async helper's body is "See the sync counterpart".

Please inline the load-bearing sentences: memory is proportional to the match count, so very large scans should partition the filter; a batch can come back smaller than batch_size; and a drained cursor isn't proof every match was seen. That last one matters most, because a method named for iteration reads as exhaustive. Keep the seen set as it is — dropping it would hand callers the same document twice, and since RediSearch reindexes an updated document under a new, higher id, writing during iteration then provokes further repeats.

Two more worth adding while you're in here. drop_by_filter and update_by_filter carry the only "never string-concatenate untrusted input into a filter" warning in the package, and these accept the same parameter type without it. And the cursor's idle timeout only resets when a page is read, so a caller doing per-key embedding or network work can have its cursor reaped after keys have already been yielded, which fails mid-stream rather than up front.

One softening, too. The MAXSEARCHRESULTS framing is accurate but reads scarier than it is. I measured the default at 1,000,000 on Redis 8.2.7, so it only bites above a million matches. Naming the figure would help a reader judge whether it applies to them.

[Copilot]

Copilot review overview

🟡 Changes recommended

Test collection is blocked by an invalid import, and review comments remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Remove invalid contextlib import causing test collection failure · New

What changed in this PR

Adds lazy, filter-based iter_keys() and aiter_keys() APIs with cursor pagination.

Changes:

• Adds synchronous and asynchronous key iterators.
• Adds integration tests for filtering, laziness, paging, and validation.
• Documents cursor behavior and cleanup semantics.

File	Summary
tests/​integration/​test_index_iteration.py	Adds integration coverage; an invalid redisvl.utils.utils import currently prevents test collection.
redisvl/​index/​index.py	Implements sync/async iterators; sync cleanup documentation and async paging/laziness validation need correction or additional coverage.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[cursor]

Stale Bugbot comment from a previous run.

[cursor]

Laziness test cannot detect eager iteration

Low Severity

test_iter_keys_is_lazy only asserts that the first yielded value is not None. Keys are strings so that always holds, and an implementation that drained the index before yielding would still pass. The test does not verify lazy or streaming behavior.

 

^{Reviewed by Cursor Bugbot for commit bf50de2. Configure here.}

[Copilot]

Copilot review overview

🟡 Changes recommended

Fix the async iterator’s aclosing misuse and address the related test issues.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Remove aclosing wrapper from list batch iteration · New
• Remove unused aclosing import · New

Resolved since last review (1)

• Remove invalid contextlib import causing test collection failure

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{Reviewed by Cursor Bugbot for commit a3c47c5. Configure here.}

[cursor]

aiter_keys acloses lists, not cursors

High Severity

aiter_keys wraps each page from _iter_keys_by_filter in aclosing, but those pages are lists and have no aclose. After the first page, iteration raises AttributeError, so a non-empty index cannot be drained. The async helper that owns the Redis cursor is left unclosed.

 

^{Reviewed by Cursor Bugbot for commit a3c47c5. Configure here.}