Python: Fix RedisJsonCollection.delete() not prefixing keys and add R…

[daric93]

Motivation and Context

Fixes #13904.

Tried to use the Redis connector and ran into issues — deletes with prefix_collection_name_to_key_names=True were silently failing, and vector search didn't work at all. The existing integration tests only cover single-record upsert → get → delete with the default prefix setting (False) and never call collection.search(), so these paths had zero test coverage. Added integration tests covering the full public surface and that's how these issues were found.

This PR fixes the JSON delete bug and adds the test coverage. The vector search issues are tracked separately in #13896 and addressed by #13899.

Description

Bug fix — RedisJsonCollection._inner_delete does not apply the collection-name prefix to keys before calling JSON.DEL. When prefix_collection_name_to_key_names=True, upsert stores keys as {collection_name}:{key} but delete sends JSON.DEL {key} (without the prefix). The command targets a non-existent key, returns 0, and the record is never deleted. The hashset sibling RedisHashsetCollection._inner_delete correctly calls self._get_redis_key(key) — the JSON version was missing it.

Test coverage — new file python/tests/integration/memory/test_redis_vector_store.py with 30 parametrised integration tests covering:

• Collection lifecycle (ensure_collection_exists, collection_exists, ensure_collection_deleted)
• list_collection_names (FT._LIST)
• Batch upsert / get / delete (both JSON and HASHSET)
• get with include_vectors=True/False
• prefix_collection_name_to_key_names=False round-trip
• Manual index creation via ensure_collection_exists(index_definition=..., fields=...)
• Error paths (ensure_collection_exists with invalid args, get without keys)
• Vector search: basic KNN, top/skip paging, include_vectors, tag/text filters (==, !=, and, or)

14 of the 30 tests exercise vector search and are marked xfail referencing #13896 / #13899 — they will pass once that PR merges and the xfail markers can be removed.

Tests require a running Redis server reachable via REDIS_CONNECTION_STRING.

Contribution Checklist

• The code builds clean without any errors or warnings
• The PR follows the SK Contribution Guidelines and the pre-submission formatting script raises no violations
• All unit tests pass, and I have added new tests where possible
• I didn't break anyone 😄

[github-actions]

Automated Code Review

Reviewers: 4 | Confidence: 92%

✓ Correctness

The one-line fix in redis.py is correct and addresses a genuine bug: RedisJsonCollection._inner_delete was not applying _get_redis_key() to keys before deletion, which would cause deletes to silently target the wrong keys when prefix_collection_name_to_key_names=True. The sibling RedisHashsetCollection._inner_delete (line 581) already applies this transform, as does RedisJsonCollection._inner_get (line 699), confirming this was an oversight. The new integration test file is well-structured and exercises the fixed code path (batch delete with prefix) along with other CRUD and search operations. No correctness issues found.

✓ Security Reliability

The one-line production fix in redis.py is correct and addresses a real bug: RedisJsonCollection._inner_delete was not applying _get_redis_key() to prefix the collection name to keys before deletion, unlike the hashset counterpart (line 581) and the JSON _inner_get (line 699). This could cause delete operations to silently fail (deleting non-existent unprefixed keys) when prefix_collection_name_to_key_names=True. The new integration test file is well-structured, uses randomized collection names to avoid collisions, and properly cleans up resources in finally blocks. No security or reliability issues found.

✓ Test Coverage

The one-line bug fix in RedisJsonCollection._inner_delete correctly adds _get_redis_key() to prefix keys, matching the pattern already used in _inner_get (line 699) and RedisHashsetCollection._inner_delete (line 581). The new integration test file is comprehensive, covering batch CRUD, include_vectors, collection lifecycle, explicit index creation, and search with filters. The fix is functionally covered by test_batch_upsert_get_delete which uses prefix_collection_name_to_key_names=True. However, there is no unit-level regression test that would have caught the original bug or prevent its reintroduction without requiring a running Redis server — the existing test_delete in test_redis_store.py only tests without prefix and doesn't assert on mock arguments.

✓ Design Approach

The one-line bug fix in RedisJsonCollection._inner_delete is correct and necessary. _inner_get (line 699) and _serialize_dicts_to_store_models (line 723) already call _get_redis_key before issuing Redis commands, and RedisHashsetCollection._inner_delete (line 581) does too. The pre-fix _inner_delete was the only Redis operation in the JSON collection that passed the raw application key instead of the prefixed Redis key, causing deletes to silently no-op whenever prefix_collection_name_to_key_names=True. The fix is consistent with the established pattern. The integration test file is reasonable: the _SEARCH_XFAIL markers are honestly documented with issue links, the fixture correctly tears down the collection inside async with col: so the Redis connection is still open during cleanup, and test_batch_upsert_get_delete correctly exercises the fixed code path with prefix_collection_name_to_key_names=True. The existing unit test test_delete (line 273-276) used collections without the prefix option and did not assert which key was passed to the mock, which is why the bug was not caught earlier — but that is a pre-existing gap, not introduced by this PR.

Suggestions

• Add a unit test in test_redis_store.py that verifies RedisJsonCollection._inner_delete calls json().delete with the prefixed key when prefix_collection_name_to_key_names=True (e.g., await collection_with_prefix_json._inner_delete(['id1']); mock_delete_json.assert_called_once_with('test:id1', '$')). The existing test_delete (line 274) only tests without prefix and doesn't assert mock arguments, which is why the original bug went unnoticed. Consider adding a symmetric test for RedisHashsetCollection._inner_delete as well.

---

Automated review by daric93's agents