feat: add COMMAND DOCS support - #3362
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f94d60805a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
| }, | ||
| transformReply: { | ||
| 2: (reply: UnwrapReply<Resp2Reply<CommandDocsReply>>) => transformDocsMap(reply), |
There was a problem hiding this comment.
When an RESP2 client uses withTypeMapping({ [RESP_TYPES.MAP]: Map }) or maps maps to Array, the public return type is adjusted to that representation, but this transform ignores the supplied typeMapping argument and always converts the top-level reply to a plain object. Code that follows the declared type, such as calling docs.get('get'), therefore fails at runtime; pass the mapping through the recursive transforms and produce the requested representation.
Useful? React with 👍 / 👎.
| deprecated_since?: BlobStringReply; | ||
| replaced_by?: BlobStringReply; | ||
| history?: ArrayReply<TuplesReply<[BlobStringReply, BlobStringReply]>>; | ||
| arguments?: ArrayReply<ReplyUnion>; |
There was a problem hiding this comment.
For TypeScript consumers, each docs.get.arguments entry is currently inferred as the broad ReplyUnion, so ordinary access such as docs.get.arguments?.[0].name is rejected even though transformArgument returns a CommandDocsArgument object and the integration test uses that shape. Type this field as an array of CommandDocsArgument entries, and make the same correction to the recursive CommandDocsArgument.arguments field.
Useful? React with 👍 / 👎.
nkaradzhov
left a comment
nkaradzhov
left a comment
There was a problem hiding this comment.
@Hashim1999164 thanks for the PR!
Three points before merge.
First, the RESP2 transformReply should honor the typeMapping parameter (see transformTuplesReply and CONFIG_GET) so users mapping MAP to Map or Array get consistent results across protocols.
Second, please add an integration test running commandDocs against a real Redis 7 server with RESP 2 client options — the nested transform is currently covered only by a synthetic fixture, which is exactly where the previous PR broke.
Third, drop the NOT_KEYED_COMMAND flag, it has no runtime consumer and the other COMMAND family commands do not set it.
Address review feedback: preserve Map/Array type mappings in the RESP2 transform, and type nested arguments as CommandDocsArgument.
|
Addressed the Codex review notes:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd3f52841d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return reply as CommandDoc; | ||
| } | ||
|
|
||
| const doc: Record<string, unknown> = {}; |
There was a problem hiding this comment.
When an RESP2 client uses withTypeMapping({ [RESP_TYPES.MAP]: Map }), the new outer switch returns a Map, but transformDoc still unconditionally creates a plain object and transformArgument does the same. RESP3 applies the mapping recursively in the decoder, so the same command returns Map<string, object> under RESP2 but Map<string, Map> under RESP3; nested maps also fail to honor the requested mapping. Fresh evidence after the earlier review comment is that the added mapping logic only changes transformDocsMap, while this allocation remains unconditional; apply the mapping consistently to document and argument maps or normalize both protocols to the same declared shape.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit fd3f528. Configure here.
| doc.subcommands = transformDocsMap(value, typeMapping); | ||
| break; | ||
| case 'reply_schema': | ||
| doc.reply_schema = Array.isArray(value) ? transformDoc(value, typeMapping) : value; |
There was a problem hiding this comment.
reply_schema misprocessed through command-doc-specific transformer
Low Severity
The reply_schema field (a JSON Schema object) is transformed via transformDoc, which applies command-doc-specific logic for keys named arguments, subcommands, and reply_schema. If the JSON Schema contains a top-level key called arguments with an array value, it gets incorrectly processed through transformArgument, which interprets each array element as a flat key-value pair list representing a CommandDocsArgument. This corrupts the schema data for any overlapping key names.
Reviewed by Cursor Bugbot for commit fd3f528. Configure here.
2 participants
Summary
COMMAND DOCS(client.commandDocs) for Redis 7+, including optional command name filters.argumentsand recursivesubcommands.@since 7.0.0).Fixes #1934
Why previous attempts were not enough
transformArgumentsAPI and did not match the RESP3-first map reply shape maintainers asked for.Test plan
parseArgscovers no args, string, and array filterstransformReply[2]nested map/arguments/subcommandsclient.commandDocs()andclient.commandDocs(['GET', 'SET'])against Redis 7+npm run check:command-jsdocand lint on changed filesNote
Low Risk
Additive read-only command support with isolated reply parsing; no changes to auth, data paths, or existing commands.
Overview
Adds
client.commandDocs/COMMAND_DOCSso callers can fetch Redis command documentation (Redis 7+), with an optional variadic filter for specific command names.The new command module builds
COMMAND DOCSviaparseCommand, marks it read-only and non-keyed, and implementstransformReply[2]to turn RESP2 flat key-value map replies into structured objects (including nestedarguments, recursivesubcommands, andreply_schema), honoringtypeMappingfor Map/Array/plain object shapes. RESP3 relies on the default decoder (transformReply[3]is unset).Registration in the commands index exposes both
COMMAND_DOCSandcommandDocswith JSDoc (@since 7.0.0). Tests cover argument parsing, RESP2 reply transformation, and integration against Redis 7+.Reviewed by Cursor Bugbot for commit fd3f528. Bugbot is set up for automated code reviews on this repo. Configure here.