Skip COMMAND GETKEYS for movablekeys commands with complete key specs

AGENTS.md

@@ -57,7 +57,7 @@ Command surface is composed via mixin classes in `redis/commands/`:
 - `sentinel.py` — sentinel-specific commands.
 - `redismodules.py` — aggregates module mixins (`bf/` Bloom, `json/`, `search/`, `timeseries/`, `vectorset/`). Each module subpackage owns its own command surface and response parsing.
 - `policies.py` — `PolicyResolver` / `StaticPolicyResolver`, the routing view the cluster client resolves per command. Also holds `STATIC_POLICIES`, which is **deprecated and unread**: a frozen verbatim copy of the 7.1.0 table, kept only for external importers and deliberately not derived from or synced with `_STATIC_COMMAND_METADATA`. Change routing in `metadata.py`, never there.
-- `metadata.py` — the command metadata record types: `RequestPolicy` / `ResponsePolicy`, `CommandPolicies` / `PolicyRecords`, and `CommandMetadata` / `CommandMetadataRecordsCache`, plus the metadata resolvers. `CommandMetadata` is the superset record; `_to_command_policies` projects one down to the routing view a `PolicyResolver` serves - projecting to `None` when the record withholds its routing policies, which is how a resolver tells the cluster client to resolve the target itself (`SCAN`, `RANDOMKEY` and the other `COMMAND_FLAGS` commands need this). The `movablekeys` reads are recorded `DEFAULT_KEYED`, which `_build_command_records` also derives from their key specs, and the cluster client finds their keys through `COMMAND GETKEYS`. The two types are kept separate deliberately: `CommandPolicies` shipped in 7.1.0 as a mutable class, and `CommandMetadata` is frozen so one record can be shared across many commands. `redis/_parsers/commands.py` re-exports the policy types from here for backwards compatibility; import them from `redis.commands.metadata`. A resolver is a client-level constructor argument (`metadata_resolver=` on `Redis`, `ConnectionPool`, `RedisCluster`, `NodesManager` and `ClusterPipeline`; on the async side only on `redis.asyncio.RedisCluster`, because the async stack has no client-side cache to be the second consumer), built once and shared with every node client; it serves cluster routing through `resolve_policies`, replica safety through `is_replica_safe`, and client-side-cache eligibility through `is_cacheable`. When only `metadata_resolver` is given the routing resolver is derived from it; an explicit `policy_resolver` still wins for the routing view - which nodes a command targets - but not for anything that view cannot express: replica safety and cache eligibility keep resolving through `metadata_resolver` either way, because a `CommandPolicies` record carries no `is_readonly` flag to answer them with.
+- `metadata.py` — the command metadata record types: `RequestPolicy` / `ResponsePolicy`, `CommandPolicies` / `PolicyRecords`, and `CommandMetadata` / `CommandMetadataRecordsCache`, plus the metadata resolvers. `CommandMetadata` is the superset record; `_to_command_policies` projects one down to the routing view a `PolicyResolver` serves - projecting to `None` when the record withholds its routing policies, which is how a resolver tells the cluster client to resolve the target itself (`SCAN`, `RANDOMKEY` and the other `COMMAND_FLAGS` commands need this). The `movablekeys` reads are recorded `DEFAULT_KEYED`, which `_build_command_records` also derives from their key specs, and the cluster client finds their keys from the key specs `CommandsParser.initialize()` compiles, asking `COMMAND GETKEYS` only where those specs are not enough. The two types are kept separate deliberately: `CommandPolicies` shipped in 7.1.0 as a mutable class, and `CommandMetadata` is frozen so one record can be shared across many commands. `redis/_parsers/commands.py` re-exports the policy types from here for backwards compatibility; import them from `redis.commands.metadata`. A resolver is a client-level constructor argument (`metadata_resolver=` on `Redis`, `ConnectionPool`, `RedisCluster`, `NodesManager` and `ClusterPipeline`; on the async side only on `redis.asyncio.RedisCluster`, because the async stack has no client-side cache to be the second consumer), built once and shared with every node client; it serves cluster routing through `resolve_policies`, replica safety through `is_replica_safe`, and client-side-cache eligibility through `is_cacheable`. When only `metadata_resolver` is given the routing resolver is derived from it; an explicit `policy_resolver` still wins for the routing view - which nodes a command targets - but not for anything that view cannot express: replica safety and cache eligibility keep resolving through `metadata_resolver` either way, because a `CommandPolicies` record carries no `is_readonly` flag to answer them with.
 - `helpers.py` — shared utilities (`list_or_args`, pubsub subscription partitioning, etc.).
 
 `Redis` (sync) and `redis.asyncio.Redis` are assembled by inheriting these mixins plus the connection/pool plumbing. Adding a new command means editing both the sync and async mixin classes and, for cluster routing, possibly the command metadata in `metadata.py`, which drives both the routing policies and replica eligibility. Not `READ_COMMANDS` - it is deprecated and unread. There is a `/add-new-command` skill (`.claude/commands/add-new-command.md`) that follows the project's expected workflow; use the spec template at `.agents/commands/add-new-command/command-specification-template.md`. Before adding commands, also read this file (AGENTS.md) and `.agents/sync_async_type_hints_overload_guide.md` — they document the sync/async type-overload convention that command additions must follow. The `/sync-claude-md` skill audits and refreshes this file when the project structure changes.

docs/resp3_features.rst

@@ -166,8 +166,10 @@ commands with the ``movablekeys`` flag. Examples are ``SINTERCARD``, ``ZDIFF``,
 - The static table lists them as keyed commands.
 - With the server metadata, the parser reads their key specs and marks them as keyed.
 
-In both cases, the Cluster client asks the server for their keys with ``COMMAND GETKEYS``
-and routes the command by those keys.
+In both cases, the Cluster client finds their keys from the key specs that ``COMMAND``
+reports and routes the command by those keys. It asks the server with ``COMMAND GETKEYS``
+only when the specs are not enough: a server older than 7.0 without key specs, a spec
+flagged ``incomplete`` or ``variable_flags``, or a keyword search.
 
 The same argument is accepted by ``ConnectionPool`` (configure it there when you supply your
 own ``connection_pool=``) and by ``RedisCluster``, which shares the one resolver with every

redis/_parsers/commands.py

@@ -20,10 +20,16 @@
     RequestPolicy,
     ResponsePolicy,
 )
-from redis.exceptions import IncorrectPolicyType, RedisError, ResponseError
+from redis.exceptions import (
+    DataError,
+    IncorrectPolicyType,
+    RedisError,
+    ResponseError,
+)
 from redis.utils import deprecated_function, str_if_bytes
 
 if TYPE_CHECKING:
+    from redis._parsers.encoders import Encoder
     from redis.asyncio.cluster import ClusterNode
 
 # The record type ``_build_command_records`` builds per command: policies or full metadata.
@@ -77,18 +83,145 @@ def _get_pubsub_keys(self, *args):
     def parse_subcommand(self, command, **options):
         return _parse_subcommand(command)
 
+    def _get_keys_from_specs(self, cmd_name, args):
+        """
+        Find the keys of a movablekeys command from the key specs compiled in
+        initialize(), the way the server does, which saves the COMMAND GETKEYS
+        round trip. Returns None when the server has to be asked.
+        """
+        compiled = self._movablekeys_specs.get(cmd_name)
+        if compiled is None:
+            return None
+        arity, specs = compiled
+        argc = len(args)
+        # COMMAND GETKEYS rejects a wrong argument count before reading specs.
+        if argc < abs(arity) or (arity > 0 and argc != arity):
+            return None
+        keys = []
+        for find_type, first, a, b, step in specs:
+            if find_type == "range":
+                lastkey, limit = a, b
+                if lastkey >= 0:
+                    last = first + lastkey
+                elif not limit:
+                    last = argc + lastkey
+                else:
+                    last = first + (argc - first) // limit + lastkey
+            else:
+                keynumidx, firstkey = a, b
+                if first + keynumidx >= argc:
+                    return None
+                numkeys = _parse_numkeys(args[first + keynumidx], self._encoder)
+                if numkeys is None or numkeys < 0:
+                    return None
+                first += firstkey
+                last = first + (numkeys - 1) * step
+            if last < first:
+                # No keys, as for the server; a negative slice end would wrap.
+                continue
+            if last >= argc:
+                return None
+            keys.extend(args[first : last + 1 : step])
+        return keys
+
+
+def _parse_numkeys(value, encoder):
+    """
+    The key count as the server reads it, or None for a value it rejects.
+    Checks the bytes the client sends: int() also accepts "+2", " 2", "02",
+    "2_0" and 2.5, and a str is sent in the client's encoding.
+    """
+    try:
+        raw = encoder.encode(value)
+    except DataError:
+        return None
+    if not isinstance(raw, (bytes, bytearray)) or not raw.isdigit():
+        return None
+    if len(raw) > 1 and raw.startswith(b"0"):
+        return None
+    # More digits than a signed 64-bit count; int() would also refuse 4300+.
+    if len(raw) > 19:
+        return None
+    return int(raw)
+
+
+def _compile_movablekeys_specs(commands):
+    """
+    Compile every movablekeys command into an ``(arity, specs)`` pair that
+    _get_keys_from_specs evaluates, or None where COMMAND GETKEYS has to be
+    asked: no specs, a spec flagged incomplete or variable_flags (the server
+    then ignores the specs too), or a search the client does not implement.
+    """
+    compiled = {}
+    for name, command in commands.items():
+        if "movablekeys" in command["flags"]:
+            specs = _compile_key_specs(command.get("key_specifications"))
+            if specs is not None:
+                specs = (int(command["arity"]), specs)
+            compiled[name] = specs
+    return compiled
+
+
+def _compile_key_specs(specs):
+    if not specs:
+        return None
+    compiled = []
+    for spec in specs:
+        flags = _key_spec_flags(spec)
+        if "incomplete" in flags or "variable_flags" in flags:
+            return None
+        if "not_key" in flags:
+            continue
+        spec = _spec_dict(spec)
+        begin = _spec_dict(spec.get("begin_search", ()))
+        find = _spec_dict(spec.get("find_keys", ()))
+        if str_if_bytes(begin.get("type")) != "index":
+            return None
+        first = _spec_dict(begin["spec"])["index"]
+        find_type = str_if_bytes(find.get("type"))
+        find_spec = _spec_dict(find.get("spec", ()))
+        step = find_spec.get("keystep", 0)
+        if first < 1 or step < 1:
+            return None
+        if find_type == "range":
+            compiled.append(
+                (find_type, first, find_spec["lastkey"], find_spec["limit"], step)
+            )
+        elif find_type == "keynum":
+            compiled.append(
+                (
+                    find_type,
+                    first,
+                    find_spec["keynumidx"],
+                    find_spec["firstkey"],
+                    step,
+                )
+            )
+        else:
+            return None
+    return tuple(compiled)
+
+
+def _spec_dict(spec):
+    # RESP2 sends a key spec as a flat [name, value, ...] list, RESP3 as a map.
+    if isinstance(spec, dict):
+        return {str_if_bytes(k): v for k, v in spec.items()}
+    return {str_if_bytes(k): v for k, v in zip(spec[::2], spec[1::2])}
+
 
 class CommandsParser(AbstractCommandsParser):
     """
     Parses Redis commands to get command keys.
     COMMAND output is used to determine key locations.
     Commands that do not have a predefined key location are flagged with
-    'movablekeys', and these commands' keys are determined by the command
-    'COMMAND GETKEYS'.
+    'movablekeys'. Their keys are read from the command's key specs, and
+    'COMMAND GETKEYS' is asked only when the specs are not enough.
     """
 
     def __init__(self, redis_connection):
         self.commands = {}
+        self._movablekeys_specs = {}
+        self._encoder = None
         self.redis_connection = redis_connection
         self.initialize(self.redis_connection)
 
@@ -101,6 +234,8 @@ def initialize(self, r):
         for cmd in uppercase_commands:
             commands[cmd.lower()] = commands.pop(cmd)
         self.commands = commands
+        self._movablekeys_specs = _compile_movablekeys_specs(commands)
+        self._encoder = r.get_encoder()
 
     # As soon as this PR is merged into Redis, we should reimplement
     # our logic to use COMMAND INFO changes to determine the key positions
@@ -140,7 +275,9 @@ def get_keys(self, redis_conn, *args):
 
         command = self.commands.get(cmd_name)
         if "movablekeys" in command["flags"]:
-            keys = self._get_moveable_keys(redis_conn, *args)
+            keys = self._get_keys_from_specs(cmd_name, args)
+            if keys is None:
+                keys = self._get_moveable_keys(redis_conn, *args)
         elif "pubsub" in command["flags"] or command["name"] == "pubsub":
             keys = self._get_pubsub_keys(*args)
         else:
@@ -255,8 +392,9 @@ class AsyncCommandsParser(AbstractCommandsParser):
     Parses Redis commands to get command keys.
 
     COMMAND output is used to determine key locations.
-    Commands that do not have a predefined key location are flagged with 'movablekeys',
-    and these commands' keys are determined by the command 'COMMAND GETKEYS'.
+    Commands that do not have a predefined key location are flagged with 'movablekeys'.
+    Their keys are read from the command's key specs, and 'COMMAND GETKEYS' is asked
+    only when the specs are not enough.
 
     NOTE: Due to a bug in redis<7.0, this does not work properly
     for EVAL or EVALSHA when the `numkeys` arg is 0.
@@ -266,17 +404,21 @@ class AsyncCommandsParser(AbstractCommandsParser):
     So, don't use this with EVAL or EVALSHA.
     """
 
-    __slots__ = ("commands", "node")
+    __slots__ = ("commands", "node", "_movablekeys_specs")
 
     def __init__(self) -> None:
         self.commands: Dict[str, Union[int, Dict[str, Any]]] = {}
+        self._movablekeys_specs: Dict[str, Optional[Tuple]] = {}
+        self._encoder: Optional["Encoder"] = None
 
     async def initialize(self, node: Optional["ClusterNode"] = None) -> None:
         if node:
             self.node = node
 
         commands = await self.node.execute_command("COMMAND")
         self.commands = {cmd.lower(): command for cmd, command in commands.items()}
+        self._movablekeys_specs = _compile_movablekeys_specs(self.commands)
+        self._encoder = self.node.get_encoder()
 
     # As soon as this PR is merged into Redis, we should reimplement
     # our logic to use COMMAND INFO changes to determine the key positions
@@ -316,7 +458,9 @@ async def get_keys(self, *args: Any) -> Optional[Tuple[str, ...]]:
 
         command = self.commands.get(cmd_name)
         if "movablekeys" in command["flags"]:
-            keys = await self._get_moveable_keys(*args)
+            keys = self._get_keys_from_specs(cmd_name, args)
+            if keys is None:
+                keys = await self._get_moveable_keys(*args)
         elif "pubsub" in command["flags"] or command["name"] == "pubsub":
             keys = self._get_pubsub_keys(*args)
         else:

redis/commands/metadata.py

@@ -1140,9 +1140,9 @@ def _load_commands_metadata_cache(
 #
 # This includes the ``movablekeys`` commands. Their ``first_key_pos`` is 0, but they do take
 # keys, so ``has_key_argument`` is True. The cluster client routes them like any other keyed
-# command: ``determine_slot`` asks the server for their keys with ``COMMAND GETKEYS``. The
-# parsers read their key specs and also mark them keyed, so a live resolver agrees with this
-# record.
+# command: ``determine_slot`` reads their keys from the ``COMMAND`` key specs, and asks the
+# server with ``COMMAND GETKEYS`` only where a spec is incomplete. The parsers also mark them
+# keyed, so a live resolver agrees with this record.
 _CACHEABLE_KEYED = CommandMetadata(
     request_policy=RequestPolicy.DEFAULT_KEYED,
     response_policy=ResponsePolicy.DEFAULT_KEYED,

redis/crc.py

@@ -15,6 +15,9 @@ def key_slot(key: EncodedT, bucket: int = REDIS_CLUSTER_HASH_SLOTS) -> int:
     :param key - bytes
     :param bucket - int
     """
+    if isinstance(key, memoryview):
+        # The encoder passes memoryviews through, and they have no find().
+        key = key.tobytes()
     start = key.find(b"{")
     if start > -1:
         end = key.find(b"}", start + 1)

tests/test_asyncio/test_cluster.py

@@ -5085,7 +5085,8 @@ class TestStaticMetadataRouting:
 
     The ``movablekeys`` reads are the sharp case, and get their own test. Their keys live only in
     the ``COMMAND`` key specifications, so ``first_key_pos`` is 0; their records carry the keyed
-    defaults, and the client finds the keys through ``COMMAND GETKEYS``.
+    defaults, and the client reads the keys from those specs, asking the server with
+    ``COMMAND GETKEYS`` only where a spec is incomplete.
 
     Key extraction itself is covered by ``tests/test_asyncio/test_command_parser.py``, so it is
     stubbed here and these tests assert only the node the command is dispatched to.