fix(search): parse RESP3 FT.SEARCH responses with bytes-typed keys (#4109)

* fix(search): parse RESP3 FT.SEARCH responses with bytes-typed keys

Since the wire protocol default switched to RESP3 (#4052), the server
returns FT.SEARCH responses as RESP3 maps.  When a client is opened with
``decode_responses=False`` the map keys arrive as ``bytes`` rather than
``str``, but ``Result.from_resp3`` looked them up as plain strings:

    instance.total = res.get("total_results", 0)
    for result_item in res.get("results", []):
        ...

Because ``b"total_results" != "total_results"``, every lookup missed and
the search appeared to return ``Result{0 total, docs: []}`` even though
the server had matched documents.

Normalise the top-level map and each per-result map to string keys
before reading them, mirroring the pattern already used by
``_parse_hybrid_search_resp3`` in ``redis/commands/search/commands.py``
("Top-level keys are normalised to strings").

Adds ``tests/test_search_result.py`` with regression tests covering
str-keyed, bytes-keyed, and mixed maps, plus the empty/None edge cases.
The tests fail on the unfixed code for the bytes and mixed cases.

Fixes #4107

* fix(search): extend bytes-key normalisation to AGGREGATE and SPELLCHECK

The RESP3 callbacks for FT.SEARCH (`Result.from_resp3`) were taught to
normalise top-level structural map keys to strings so that responses
parsed correctly on connections opened with `decode_responses=False`.
`_parse_aggregate_resp3` (FT.AGGREGATE / FT.CURSOR READ /
FT.PROFILE AGGREGATE) and `_parse_spellcheck_resp3` (FT.SPELLCHECK)
still read `"total_results"`, `"results"` and `"warning"` as
plain strings, so a byte-keyed RESP3 response missed every lookup and
silently parsed as an empty AggregateResult / `{}` even when the
server had returned data.

Apply the same `str_if_bytes` normalisation that
`Result.from_resp3` and `_parse_hybrid_search_resp3` already use:

- normalise the top-level map and (for aggregate) the per-result-item
  map; document data inside `extra_attributes` is left as-is so the
  caller still sees bytes when `decode_responses=False`, mirroring the
  RESP2 shape;
- normalise the outer `results` key for spellcheck; the inner term keys
  match the RESP2 `decode_responses=False` shape and stay as bytes.

Adds regression tests for both parsers in
`tests/test_search_result.py`, plus integration tests in
`tests/test_search.py` that exercise the three affected Search
callbacks (FT.SEARCH, FT.AGGREGATE, FT.SPELLCHECK) against a real
RESP3 wire with a `decode_responses=False` client.

* fix(search): apply petyaslavova review feedback

- _parse_spellcheck_resp3 now preserves the suggestion value as-is so
  it keeps the decode_responses shape RESP2 would produce (str when
  decoded, bytes otherwise) instead of wrapping bytes in str().
- waitForIndex now accepts both str and bytes structural keys in
  FT.INFO responses.  execute_command bypasses the search module's
  callbacks, so the helper has to handle the raw RESP3 dict/RESP2
  list shapes for decode_responses=False clients.  This unblocks the
  previously failing fixed-clients CI matrix entry.
- The bytes-keys integration tests are now parametrised over
  protocol=2 (anchors the legacy output shape) and the default
  protocol (the path that actually exercises the changed parsers in
  _RedisCallbacksRESP3toRESP2Legacy).  Explicit protocol=3 was
  routing through _RedisCallbacksRESP3 and bypassing the fix.
- Spellcheck assertion is stricter: it pins the term key to
  b"impornant" and the suggestion value to b"important".
- Mirror the suggestion-bytes assertion in the
  test_search_result.py unit test.

* test(search): tidy review nits in RESP3 bytes-key tests

- Rephrase the parametrisation comment so it explains *why* the two
  protocol arms exist in terms of the parsers being exercised
  (_parse_search_resp3 / _parse_aggregate_resp3 /
  _parse_spellcheck_resp3) rather than "the changed methods", which
  was only meaningful relative to this PR's diff.
- Reorder decorators on TestSearchResp3BytesKeys methods so the
  test-scope marks (redismod, fixed_client) stay grouped and
  @pytest.mark.parametrize sits last, matching the prevailing style
  for parametrised tests.

---------

Co-authored-by: petyaslavova <petya.slavova@redis.com>

Author: mokashang

redis/commands/search/commands.py

@@ -494,11 +494,19 @@ def _parse_aggregate_resp3(self, res, **kwargs):
         else:
             data = res
 
+        if data is None:
+            data = {}
+        # On RESP3 connections with decode_responses=False the server's map
+        # keys arrive as bytes, so normalise structural keys to strings
+        # before lookup.  Mirrors ``Result.from_resp3``.
+        data = {str_if_bytes(k): v for k, v in data.items()}
+
         warnings = [str_if_bytes(w) for w in data.get("warning", [])]
         total = data.get("total_results", 0)
 
         rows = []
         for result_item in data.get("results", []):
+            result_item = {str_if_bytes(k): v for k, v in result_item.items()}
             extra_attrs = result_item.get("extra_attributes", {})
             # Convert dict to flat list [key, value, key, value, ...]
             # to match RESP2 row format consumers expect.
@@ -640,6 +648,10 @@ def _parse_spellcheck_resp3(self, res, **kwargs):
         """
         if not isinstance(res, dict):
             return self._parse_spellcheck(res, **kwargs)
+        # On RESP3 connections with decode_responses=False the server's map
+        # keys arrive as bytes, so normalise the structural ``results`` key
+        # to a string before lookup.  Mirrors ``Result.from_resp3``.
+        res = {str_if_bytes(k): v for k, v in res.items()}
         corrections = {}
         results = res.get("results", {})
         for term, suggestions in results.items():
@@ -654,8 +666,11 @@ def _parse_spellcheck_resp3(self, res, **kwargs):
                     score_str = str(score)
                     if score_str.endswith(".0"):
                         score_str = score_str[:-2]
+                    # Preserve ``suggestion`` as-is so it keeps the
+                    # ``decode_responses`` shape RESP2 would produce
+                    # (``str`` when decoded, ``bytes`` otherwise).
                     term_corrections.append(
-                        {"score": score_str, "suggestion": str(suggestion)}
+                        {"score": score_str, "suggestion": suggestion}
                     )
             if term_corrections:
                 corrections[term] = term_corrections

redis/commands/search/result.py

@@ -111,12 +111,17 @@ def from_resp3(
         instance = cls.__new__(cls)
         if res is None:
             res = {}
+        # On RESP3 connections with decode_responses=False the server's map
+        # keys arrive as bytes, so normalise them to strings before lookup
+        # to keep behaviour consistent with decode_responses=True.
+        res = {str_if_bytes(k): v for k, v in res.items()}
         instance.total = res.get("total_results", 0)
         instance.duration = duration
         instance.docs = []
         instance.warnings = [str_if_bytes(w) for w in res.get("warning", [])]
 
         for result_item in res.get("results", []):
+            result_item = {str_if_bytes(k): v for k, v in result_item.items()}
             doc_id = str_if_bytes(result_item.get("id", ""))
             score = None
             if with_scores and "score" in result_item:

tests/test_search.py

@@ -43,7 +43,7 @@
 )
 from redis.commands.search.result import Result
 from redis.commands.search.suggestion import Suggestion
-from redis.utils import safe_str
+from redis.utils import SENTINEL, safe_str
 
 from .conftest import (
     _get_client,
@@ -84,15 +84,28 @@ def waitForIndex(env, idx, timeout=None):
         while True:
             try:
                 res = env.execute_command("FT.INFO", idx)
-                if int(res[res.index("indexing") + 1]) == 0:
+                # ``execute_command`` bypasses the search module's
+                # callbacks, so the response is the raw wire shape.
+                # With ``decode_responses=False`` the structural keys
+                # arrive as bytes; accept both forms.
+                try:
+                    i = res.index("indexing")
+                except ValueError:
+                    i = res.index(b"indexing")
+                if int(res[i + 1]) == 0:
                     break
             except ValueError:
                 break
             except AttributeError:
+                # RESP3 dict response.  Keys may be ``str`` or ``bytes``
+                # depending on ``decode_responses``.
+                indexing = res.get("indexing")
+                if indexing is None:
+                    indexing = res.get(b"indexing")
                 try:
-                    if int(res["indexing"]) == 0:
+                    if int(indexing) == 0:
                         break
-                except ValueError:
+                except (TypeError, ValueError):
                     break
             except ResponseError:
                 # index doesn't exist yet
@@ -5590,3 +5603,115 @@ def test_hybrid_search_query_with_multiple_loads_and_applies(self, client):
                 assert item["description"] is not None
                 assert item["discount_10_percents"] is not None
                 assert item["additional_discount"] is not None
+
+
+# Parametrise the bytes-key regression tests over RESP2 and the
+# default protocol.  RESP2 uses ``_RedisCallbacksRESP2`` and anchors the
+# expected legacy output shape with ``decode_responses=False``.  The
+# default protocol (``SENTINEL`` -> not specified) leaves the wire on
+# RESP3 with the ``_RedisCallbacksRESP3toRESP2Legacy`` adapter selected,
+# which is where the bytes-key normalisation in ``_parse_search_resp3``,
+# ``_parse_aggregate_resp3`` and ``_parse_spellcheck_resp3`` lives.  An
+# explicit ``protocol=3`` would route through ``_RedisCallbacksRESP3``
+# instead and bypass the methods we want to test.
+_SEARCH_BYTES_PROTOCOLS = [
+    pytest.param(2, id="resp2"),
+    pytest.param(SENTINEL, id="default-resp3"),
+]
+
+
+def _make_bytes_search_client(request, stack_url, protocol):
+    kwargs = {
+        "decode_responses": False,
+        "from_url": stack_url,
+    }
+    if protocol is not SENTINEL:
+        kwargs["protocol"] = protocol
+    client = _get_client(redis.Redis, request, **kwargs)
+    client.flushdb()
+    return client
+
+
+class TestSearchResp3BytesKeys(SearchTestsBase):
+    """Regression tests for #4107.
+
+    With the default protocol (RESP3 on the wire) and
+    ``decode_responses=False`` the server's structural map keys arrive
+    as ``bytes`` (e.g. ``b"results"`` rather than ``"results"``).  The
+    RESP3->legacy-RESP2 search callbacks used to look those keys up as
+    plain strings, missed them, and silently produced empty results.
+    Each test is parametrised over ``protocol=2`` (anchors the legacy
+    output shape) and the default protocol (exercises the actual fixed
+    parsers in ``_RedisCallbacksRESP3toRESP2Legacy``).
+    """
+
+    @pytest.mark.redismod
+    @pytest.mark.fixed_client
+    @pytest.mark.parametrize("protocol", _SEARCH_BYTES_PROTOCOLS)
+    def test_search_resp3_bytes_keys(self, request, stack_url, protocol):
+        client = _make_bytes_search_client(request, stack_url, protocol)
+        client.ft().create_index((TextField("title"), TextField("body")))
+        client.hset("doc1", mapping={"title": "hello", "body": "redis world"})
+        client.hset("doc2", mapping={"title": "hello", "body": "search world"})
+        self.waitForIndex(client, getattr(client.ft(), "index_name", "idx"))
+
+        res = client.ft().search(Query("hello"))
+
+        # Before the fix the default-RESP3 case returned
+        # ``Result{0 total, docs: []}`` because ``Result.from_resp3``
+        # looked up the bytes-keyed map by ``str`` keys.  ``Result``
+        # always normalises the doc id with ``str_if_bytes`` so the id
+        # assertion stays in ``str`` form even with
+        # ``decode_responses=False``.
+        assert res.total == 2
+        assert {d.id for d in res.docs} == {"doc1", "doc2"}
+
+    @pytest.mark.redismod
+    @pytest.mark.fixed_client
+    @pytest.mark.parametrize("protocol", _SEARCH_BYTES_PROTOCOLS)
+    def test_aggregate_resp3_bytes_keys(self, request, stack_url, protocol):
+        client = _make_bytes_search_client(request, stack_url, protocol)
+        client.ft().create_index((TextField("title"), TextField("parent")))
+        client.hset("doc1", mapping={"title": "alpha", "parent": "redis"})
+        client.hset("doc2", mapping={"title": "beta", "parent": "redis"})
+        client.hset("doc3", mapping={"title": "gamma", "parent": "redis"})
+        self.waitForIndex(client, getattr(client.ft(), "index_name", "idx"))
+
+        req = aggregations.AggregateRequest("redis").group_by(
+            "@parent", reducers.count()
+        )
+        res = client.ft().aggregate(req)
+
+        # Before the fix the default-RESP3 case missed
+        # ``data.get("total_results")`` and ``data.get("results")``
+        # because the keys were ``bytes``, yielding ``total=0`` and
+        # ``rows=[]``.
+        assert len(res.rows) == 1
+        row = res.rows[0]
+        # Row content stays as bytes (matches RESP2 with
+        # decode_responses=False); only the structural map keys are
+        # normalised.
+        assert b"parent" in row
+        assert b"redis" in row
+
+    @pytest.mark.redismod
+    @pytest.mark.fixed_client
+    @pytest.mark.parametrize("protocol", _SEARCH_BYTES_PROTOCOLS)
+    def test_spellcheck_resp3_bytes_keys(self, request, stack_url, protocol):
+        client = _make_bytes_search_client(request, stack_url, protocol)
+        client.ft().create_index((TextField("f1"),))
+        client.hset("doc1", mapping={"f1": "some valid content"})
+        client.hset("doc2", mapping={"f1": "very important"})
+        self.waitForIndex(client, getattr(client.ft(), "index_name", "idx"))
+
+        res = client.ft().spellcheck("impornant")
+
+        # Before the fix the default-RESP3 case had
+        # ``res.get("results", {})`` miss the ``b"results"`` key and
+        # return an empty ``{}``.  Both protocols carry the term and
+        # suggestion through as bytes, matching
+        # ``_parse_spellcheck`` with ``decode_responses=False``.
+        assert b"impornant" in res
+        suggestions = res[b"impornant"]
+        assert suggestions
+        assert suggestions[0]["suggestion"] == b"important"