feat(executor): parse RESP3 map replies for FT.SEARCH and FT.AGGREGATE

.github/workflows/test.yml

@@ -46,3 +46,53 @@ jobs:
         run: |
           make test
 
+
+  redis-py-compat:
+    name: redis-py ${{ matrix.redis-py }}
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        redis-py: ["6.3.0", "7.4.0", "8.1.0"]
+
+    steps:
+      - name: Check out repository
+        uses: actions/checkout@v6
+
+      - name: Install Python
+        uses: actions/setup-python@v6
+        with:
+          python-version: "3.12"
+
+      - name: Install uv
+        uses: astral-sh/setup-uv@v6
+        with:
+          version: ${{ env.UV_VERSION }}
+          enable-cache: true
+          python-version: "3.12"   # sets UV_PYTHON
+          cache-dependency-glob: |
+            pyproject.toml
+            uv.lock
+
+      - name: Install dependencies
+        run: |
+          uv sync
+
+      # Without this, a resolution surprise leaves the job exercising the
+      # locked redis-py while still reporting green, and the axis rots.
+      - name: Confirm the redis-py overlay took effect
+        run: >
+          uv run --with 'redis==${{ matrix.redis-py }}'
+          python -c "import redis, sys;
+          print('redis-py', redis.__version__);
+          sys.exit(0 if redis.__version__ == '${{ matrix.redis-py }}' else 1)"
+
+      # Reply shapes depend on the redis-py version and the negotiated RESP
+      # protocol, so the protocol-marked tests run against each supported
+      # redis-py. Selecting by marker rather than by filename means a new
+      # protocol-sensitive test is picked up here without editing this job.
+      # 'uv run --with' layers an ephemeral overlay and never writes uv.lock.
+      - name: Run reply-shape and protocol tests
+        run: >
+          uv run --with 'redis==${{ matrix.redis-py }}'
+          python -m pytest -m protocol -q

docs/concepts/index.md

@@ -60,7 +60,7 @@ Foundational reading for sql-redis. Each page explains a single design choice or
 
     ---
 
-    What `QueryResult.rows` actually contains, why it varies with the command, scoring, and client decoding.
+    What `QueryResult.rows` actually contains, why it varies with the command, scoring, client decoding and the wire protocol.
 
 -   :material-test-tube:{ .lg .middle } **[Testing philosophy](testing-philosophy.md)**
 

docs/concepts/result-shape.md

@@ -4,14 +4,16 @@ Every `Executor.execute()` call returns a `QueryResult` with two attributes: `ro
 
 ## What `count` means
 
-The integer in `count` is what Redis returned at position 0 of the reply. Its meaning depends on which command ran (see [FT.SEARCH vs FT.AGGREGATE](search-vs-aggregate.md)):
+`count` is whatever Redis reported for the query: position 0 of a RESP2 array reply, or `total_results` in a RESP3 map reply. Its meaning depends on which command ran (see [FT.SEARCH vs FT.AGGREGATE](search-vs-aggregate.md)):
 
-- **`FT.SEARCH`** (no aggregation): `count` is the **total number of matching documents** in the index, regardless of `LIMIT`. So `count` can be much larger than `len(rows)`. This is useful for pagination.
-- **`FT.AGGREGATE`** (any aggregation, GROUP BY, computed field, date function): `count` is the **number of rows in the reply**, which is what you got back. After `LIMIT`, the two are equal.
+- **`FT.SEARCH`** (no aggregation): `count` is the **total number of matching documents**, regardless of `LIMIT`. So `count` can be much larger than `len(rows)`, which is what makes it useful for pagination. Both protocols report the same figure.
+- **`FT.AGGREGATE`** (any aggregation, GROUP BY, computed field, date function): `count` is **not the number of rows you got back**, and it is the one value that differs by protocol. With a `GROUP BY`, both protocols report the number of groups before `LIMIT`. Without one, a computed-field projection leaves RESP2 reporting `1` (a placeholder the [Redis docs](https://redis.io/docs/latest/commands/ft.aggregate/) call "not a valid value") while RESP3 reports the real figure. Use `len(rows)` if you want the rows you received.
+
+Neither aggregate value can be derived from the other without changing what `protocol=2` callers already get, so the library reports what Redis sent.
 
 ## What `rows` looks like
 
-A row is always a `dict`, but the *keys* and *values* depend on three orthogonal factors.
+A row is always a `dict`, but the *keys* and *values* depend on four orthogonal factors.
 
 ### Factor 1: client decoding
 
@@ -50,24 +52,31 @@ If your SELECT contains `score()`, the underlying `FT.SEARCH` runs with `WITHSCO
 
 ```python
 # SELECT title, score() AS relevance FROM products WHERE fulltext(...)
-{b"title": b"gaming laptop", b"relevance": "0.5"}
+{b"title": b"gaming laptop", b"relevance": b"0.5"}   # RESP2
+{b"title": b"gaming laptop", b"relevance": 0.5}      # RESP3
 ```
 
 The score column name is whatever alias you wrote (`score() AS relevance` produces `relevance`). If you used `score()` without `AS`, the library falls back to a stable internal name.
 
-The score's value type is `str` (or `bytes` depending on decoding); if you need a `float`, convert at the call site.
+The score's value type depends on the wire protocol: `str` (or `bytes`, depending on decoding) on RESP2, and `float` on RESP3, where Redis sends it as a double. `float()` is the protocol-safe conversion at the call site and works on both.
 
 ### Factor 4: `RETURN 0`
 
 If your SELECT is just `score()` with no document fields, the underlying command uses `RETURN 0` (no document fields returned). Each row contains only the score column:
 
 ```python
 # SELECT score() AS s FROM products WHERE fulltext(title, 'laptop')
-{b"s": "0.5"}
+{b"s": b"0.5"}
 ```
 
 This is mostly useful when you want a relevance-only ranking without paying to ship the document body back.
 
+## A note on the wire protocol
+
+Redis replies to `FT.SEARCH` and `FT.AGGREGATE` with a flat array on RESP2 and a map on RESP3, and `sql_redis` reads the raw reply, so the protocol your client negotiated decides which shape arrives. The library parses both and the rows come out the same, but two values differ: the score's type (Factor 3) and the aggregate `count` described above.
+
+Worth knowing because redis-py 8 defaults to RESP3 where earlier versions defaulted to RESP2, so an upgrade can change both without you passing `protocol` at all.
+
 ## Score-column collision avoidance
 
 What happens if your document has a field literally named `__score`? The library detects the collision and renames the score column. The deterministic fallback is `__score_<alias>`. So:
@@ -81,7 +90,7 @@ You will not normally see this; it is a defensive measure for the unusual case.
 
 It would be tempting to normalise everything: always strings, always `float` scores, always uniform shape. The library does not, for three reasons.
 
-1. **Bytes is the right default for Redis.** Some fields legitimately contain binary data. Force-decoding to UTF-8 would corrupt those.
+1. **Bytes is the right default for Redis.** Some fields legitimately contain binary data. Force-decoding to UTF-8 would corrupt those. The same argument covers the RESP3 score: stringifying a double cannot reproduce the text Redis would have sent on RESP2, so normalising would invent a third form rather than deliver parity.
 2. **The shape difference between `FT.SEARCH` and `FT.AGGREGATE` is intrinsic to the Redis commands.** Hiding it would lie about what is happening.
 3. **One-call-site logic is cheap; library-wide policy is expensive.** A user who wants a uniform shape can write a 5-line decoder once, customised to their data. The library shipping that decoder for everyone is the wrong place for the policy.
 

docs/concepts/search-vs-aggregate.md

@@ -39,6 +39,6 @@ These constraints are not bugs to fix; they are the cost of the abstraction. The
 Two practical consequences:
 
 1. **The result-row shape changes.** `FT.SEARCH` returns rows with field-value pairs straight from the indexed documents, possibly with a score column. `FT.AGGREGATE` returns rows of computed fields, group keys, and reduced values; the original document fields are present only if the SQL asks for them. See [Result shape](result-shape.md).
-2. **`LIMIT` semantics differ subtly.** Both commands honour `LIMIT`, but the `count` returned by `FT.AGGREGATE` reflects the post-pipeline row count, while `FT.SEARCH`'s `count` is the total match count regardless of the limit. The library exposes both as `QueryResult.count`; the meaning depends on which path ran.
+2. **`LIMIT` semantics differ subtly.** Both commands honour `LIMIT`, but `FT.SEARCH`'s `count` is the total match count regardless of the limit, while `FT.AGGREGATE`'s is neither the match count nor the row count, and is the one value that differs between RESP2 and RESP3. The library exposes both as `QueryResult.count`; see [Result shape](result-shape.md#what-count-means) before relying on it.
 
 If you need to know which command was issued for a given SQL, call `Translator.translate(sql)` directly and inspect `TranslatedQuery.command`.

docs/for-ais-only/BUILD_AND_TEST.md

@@ -62,7 +62,11 @@ uv run --group docs mkdocs build --strict
 
 ## CI gates (target state)
 
-- `make check` (lint + tests) on every PR.
+- `make check` (lint + tests) on every PR, across Python 3.9 to 3.13.
+- `pytest -m protocol` against redis-py 6.3.0, 7.4.0 and 8.1.0 (the
+  `redis-py-compat` job). Mark any test whose result depends on the RESP
+  protocol or the redis-py version with `@pytest.mark.protocol` so it runs
+  there.
 - `make docs-build` with `-W` on every PR.
 - 100% coverage gate.
 
@@ -76,7 +80,18 @@ uv run pytest tests/test_parser.py tests/test_translator.py -x -vv
 
 These do not need Redis and run in well under a second.
 
-When changing executor logic:
+When changing reply parsing:
+
+```
+uv run pytest tests/test_reply_shapes.py tests/test_nil_field_array.py -x -vv
+```
+
+These feed canned replies through the parser with no server, so they are the
+fastest signal on a parsing change. Follow with
+`tests/test_protocol_matrix.py`, which asserts the same behaviour against a
+real server on both protocols.
+
+When changing other executor logic:
 
 ```
 uv run pytest tests/test_executor.py tests/test_sql_queries.py -x -vv

docs/for-ais-only/FAILURE_MODES.md

@@ -13,6 +13,33 @@ not "fix" this in `executor.py` by force-decoding; that breaks users who
 deliberately want bytes (binary fields, vectors). See
 [Result shape](../concepts/result-shape.md) for the full story.
 
+## Reply shape depends on the RESP protocol, not on library configuration
+
+`sql_redis` sends FT.* commands with `client.execute_command` and reads the raw
+reply. redis-py registers no `FT.SEARCH` / `FT.AGGREGATE` response callback on
+the base client. `Search.search()` calls `execute_command` and then its own
+`_parse_results`, so the executor receives the wire shape verbatim: a flat
+array on RESP2, a map on RESP3.
+
+`_parse_reply` therefore dispatches on the shape of the reply
+(`isinstance(raw_result, dict)`), and `_fold_map_to_array` folds a map back
+into the array shape so one parser handles both. Do not "simplify" this into a
+single unconditional path, and do not replace the shape check with a protocol
+check on the connection: sniffing `connection_kwargs["protocol"]` is wrong for
+cluster and sentinel clients, and it cannot distinguish redis-py 8's four
+callback modes (`protocol=2`, protocol unset, `protocol=3`,
+`legacy_responses=False`).
+
+Routing through `client.ft(index)` does not help. On redis-py 7.4 and earlier
+`_parse_results` returns the raw map unchanged under RESP3, and on redis-py 8
+the `protocol=3` callback table has no `FT.AGGREGATE` entry at all.
+
+Structural map keys (`total_results`, `results`, `id`, `score`,
+`extra_attributes`) arrive as `bytes` when the client has
+`decode_responses=False`, which is why `_map_get` probes both key types. Reading
+only `str` keys fails silently with zero rows rather than raising; that is
+exactly the bug redis-py shipped in 8.0.0 (redis-py#4107).
+
 ## Stopwords are silently stripped
 
 `WHERE title = 'bank of america'` becomes `@title:"bank america"` because

docs/for-ais-only/REPOSITORY_MAP.md

@@ -69,7 +69,7 @@ tests/
 | `IS NULL`, `IS NOT NULL`, `exists()` | `parser.py`, `query_builder.py` |
 | Parameter substitution | `executor.py::_substitute_params` |
 | FT.SEARCH vs FT.AGGREGATE branching | `translator.py::translate_parsed` (the `use_aggregate` boolean) |
-| Result-row parsing | `executor.py::Executor.execute` (the four parsing branches: WITHSCORES + RETURN 0, WITHSCORES, plain SEARCH, AGGREGATE) |
+| Result-row parsing | `executor.py::_parse_reply` (dispatches by reply shape), `_parse_array_reply` (the four branches: WITHSCORES + RETURN 0, WITHSCORES, plain SEARCH, AGGREGATE), `_fold_map_to_array` (folds a RESP3 map into the array shape) |
 
 ## What to read before changing X
 
@@ -79,9 +79,10 @@ tests/
 - **Analyzer/query-builder changes.** Read `analyzer.py` and the matching
   branch in `query_builder.py`. The two are tightly coupled: a new field type
   in the analyzer needs a corresponding emitter in the builder.
-- **Executor changes.** The sync and async executors share `_ScoreParseMixin`
-  and the result-parsing branches. If you change the response shape, both
-  need updates.
+- **Executor changes.** The sync and async executors share the module-level
+  reply parsers, so a parsing change lands once. Their `execute()` bodies are
+  still near-identical above the `_parse_reply` call, so a change there needs
+  applying twice.
 - **Schema registry changes.** The two registry classes do not share code,
   but they share semantics. Lazy loading, invalidation, and load coalescing
   must be consistent across sync and async.

docs/for-ais-only/index.md

@@ -26,7 +26,7 @@ Use this to find the right starting point fast.
 | Change schema loading semantics | [Schema-aware translation](../concepts/schema-aware-translation.md), then `schema.py` |
 | Change async behavior | [Async invariants](../concepts/async-invariants.md), then `executor.py::AsyncExecutor` and `schema.py::AsyncSchemaRegistry` |
 | Change the FT.SEARCH/FT.AGGREGATE branching | [FT.SEARCH vs FT.AGGREGATE](../concepts/search-vs-aggregate.md), then `translator.py::translate_parsed` |
-| Change the result-row shape | [Result shape](../concepts/result-shape.md), then `executor.py` (`Executor.execute` parsing branches) |
+| Change the result-row shape | [Result shape](../concepts/result-shape.md), then `executor.py::_parse_reply` and the functions below it |
 | Run tests | [Build and test](BUILD_AND_TEST.md) |
 | Diagnose "this looks broken" | [Failure modes](FAILURE_MODES.md) before assuming a bug |
 

pyproject.toml

@@ -82,6 +82,9 @@ exclude = '''
 [tool.pytest.ini_options]
 log_cli = true
 asyncio_mode = "auto"
+markers = [
+    "protocol: reply shapes depend on the RESP protocol and the redis-py version (run by the redis-py-compat CI job)",
+]
 
 [tool.mypy]
 warn_unused_configs = true