feat(search): add WITHSCORES support to FT.SEARCH (#2143) - #3432
feat(search): add WITHSCORES support to FT.SEARCH (#2143)#3432watersRand wants to merge 6 commits into
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: 7741133c79
ℹ️ 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".
| if(typeof reply[i] === 'number' || (typeof reply[i] === 'string' && !isNaN(Number(reply[i])) && Array.isArray(reply[i + 1]))){ | ||
| score = Number(reply[i++]); |
There was a problem hiding this comment.
For RESP2, FT.SEARCH ... WITHSCORES returns each row as id, score, fields, but this checks for and consumes a score before consuming the ID. A reply such as [1, 'doc', '1', ['field', 'value']] is therefore parsed with '1' as the document value and then produces a spurious row. Moreover, without WITHSCORES, a numeric document ID in [1, '1', ['field', 'value']] is mistaken for a score, corrupting ordinary RESP2 searches as well; consume the ID first and determine score presence from the requested reply layout rather than the ID's numeric shape.
Useful? React with 👍 / 👎.
| VERBATIM?: boolean; | ||
| NOSTOPWORDS?: boolean; | ||
| INKEYS?: RedisVariadicArgument; | ||
| WITHSCORES?: boolean; |
There was a problem hiding this comment.
SEARCH_NOCONTENT.parseCommand reuses Parameters<typeof SEARCH.parseCommand>, so adding WITHSCORES here also exposes and serializes it for ft.searchNoContent. That command's separate RESP2 transformer returns reply.slice(1), treating every score as another document ID, while its RESP3 transformer maps the main result back to IDs and silently discards every score. Callers can therefore request scores through the public type but cannot receive a valid no-content result; either exclude this option from that variant or preserve ID/score pairs in its reply contract.
Useful? React with 👍 / 👎.
| const rawScore = getMapValue(resultMap,['score']); | ||
| const score = rawScore !== undefined ? Number(rawScore) : undefined; |
There was a problem hiding this comment.
When a caller uses withTypeMapping({ [RESP_TYPES.DOUBLE]: String }), the RESP3 decoder supplies the score as a string, but this unconditional Number conversion changes it back to a number, so the newly exposed field ignores the client's requested reply mapping. Scored commands normally pass RESP3 doubles through and use transformDoubleReply[2] for RESP2; this field should follow the same pattern and use a DoubleReply-compatible type so the runtime value and inferred type remain consistent.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?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: d226bdb672
ℹ️ 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".
| _preserve?: any, | ||
| _typeMapping?: TypeMapping | ||
| _typeMapping?: TypeMapping, | ||
| options?: FtSearchOptions |
There was a problem hiding this comment.
Although the prior parser-order issue is addressed, the new option-aware branch still cannot execute through the client: command transformers are called with only (reply, parser.preserve, typeMapping), parseCommand never assigns the search options to parser.preserve, and this fourth parameter is therefore always undefined. On RESP2, requests using WITHSCORES, NOCONTENT, WITHPAYLOADS, or WITHSORTKEYS consequently parse every optional element as document content or another ID; preserve the relevant options during command parsing and consume them as the transformer's second argument.
Useful? React with 👍 / 👎.
| id: String((id as { toString?(): string })?.toString?.() ?? id ?? ''), | ||
| ...(score !== undefined && !isNaN(score) ? {score} : {}), | ||
| ...(scoreExplain !== undefined ? {scoreExplain} : {}), | ||
| ...(rawPayload !== undefined ? {payload: String(rawPayload)} : {}), |
There was a problem hiding this comment.
When WITHPAYLOADS is used for an ordinary document with no payload, Redis returns null and this expression exposes the string "null"; with a blob-string-to-Buffer type mapping, it also decodes arbitrary payload bytes through UTF-8 instead of preserving the Buffer. This makes RESP3 behavior inconsistent with RESP2 and corrupts binary payloads, so the raw null/blob value should be retained and the reply type should include those possibilities.
Useful? React with 👍 / 👎.
| if (options?.EXPLAINSCORE) { | ||
| parser.push('EXPLAINSCORE'); |
There was a problem hiding this comment.
When a caller supplies { EXPLAINSCORE: true }, which the new options interface permits independently, the parser emits EXPLAINSCORE without WITHSCORES. Redis requires WITHSCORES for this modifier, so this validly typed invocation fails at the server instead of returning a search reply; either add WITHSCORES automatically or reject/encode the invalid combination in the public API.
Useful? React with 👍 / 👎.
|
Hi @watersRand, thanks for taking this! It looks like there are some outstanding comments from the bots. If you need any help, let me know! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf79ad9750
ℹ️ 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".
| if (Array.isArray(rawScore)){ | ||
| score = Number(rawScore[0]); | ||
| if (Array.isArray(rawScore[1])){ | ||
| scoreExplain = rawScore[1].map(String); |
There was a problem hiding this comment.
When EXPLAINSCORE returns a normal multi-level scorer explanation, nested child arrays are coerced with String, collapsing each subtree into comma-joined text and losing its hierarchy. The RESP2 path retains the nested arrays, so the same query also produces incompatible shapes across protocols; preserve the recursive explanation structure and type it accordingly.
Useful? React with 👍 / 👎.
| ...(score !== undefined && !isNaN(score) ? {score} : {}), | ||
| ...(scoreExplain !== undefined ? {scoreExplain} : {}), | ||
| ...(rawPayload !== undefined && rawPayload !== null? {payload: String(rawPayload)} : {}), | ||
| ...(rawSortKey !== undefined && rawSortKey !== null ? {sortKey: String(rawSortKey)} : {}), |
There was a problem hiding this comment.
With RESP3 and withTypeMapping({ [RESP_TYPES.BLOB_STRING]: Buffer }), rawSortKey is a Buffer, but String(rawSortKey) always converts it back to a UTF-8 string. Consequently WITHSORTKEYS ignores the caller's requested mapping and can corrupt non-UTF-8 sortable values; retain the mapped reply value and expose a BlobStringReply-compatible type.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ebac4f0d9
ℹ️ 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".
| const hasExplain = Boolean(options?.EXPLAINSCORE) || Boolean(options?.EXPLAINSCORE); | ||
| const hasPayloads = Boolean(options?.WITHPAYLOADS); | ||
| const hasSortKeys = Boolean(options?.WITHSORTKEYS); | ||
| const noContent = Boolean(options?.NOCONTENT); |
There was a problem hiding this comment.
On RESP2, RETURN: [] is encoded as RETURN 0, which returns only document IDs just like NOCONTENT, but noContent remains false here. With at least two matches, the loop consumes the second ID as the first document's value and drops/misaligns the remaining results, regressing the existing RETURN: [] behavior; include an empty RETURN list when determining the reply layout.
Useful? React with 👍 / 👎.
| const hasScores = Boolean(options?.WITHSCORES); | ||
| const hasExplain = Boolean(options?.EXPLAINSCORE) || Boolean(options?.EXPLAINSCORE); |
There was a problem hiding this comment.
For the valid typed call { EXPLAINSCORE: true }, the new argument logic now automatically emits WITHSCORES, but this reply-layout flag still checks only the user-supplied WITHSCORES property. On RESP2 the returned score/explanation tuple is therefore consumed as document content and subsequent fields become spurious documents; the fresh automatic WITHSCORES behavior needs to be mirrored here by treating EXPLAINSCORE as scored output.
Useful? React with 👍 / 👎.
| parser.push('FT.SEARCH', index, query); | ||
|
|
||
| parseSearchOptions(parser, options); | ||
| parser.preserve = options; |
There was a problem hiding this comment.
This new preservation assignment only runs through SEARCH.parseCommand, while PROFILE_SEARCH.parseCommand calls parseSearchOptions directly and later delegates its RESP2 results to this option-dependent transformer (PROFILE_SEARCH.ts:116 and :126). Consequently profileSearch with WITHSCORES, NOCONTENT, payloads, or sort keys still parses its RESP2 result using the plain id, fields layout; the fresh evidence beyond the earlier preservation issue is that the wrapper path never executes this assignment, so it also needs to preserve its search options.
Useful? React with 👍 / 👎.
| SEARCH.parseCommand(parser, index, query, options as FtSearchOptions); | ||
| parser.push('NOCONTENT'); |
There was a problem hiding this comment.
On RESP3 connections to module versions that return the legacy array-shaped search reply, SEARCH_NOCONTENT.transformReply[3] intentionally falls back through SEARCH.transformReply[3], but this call preserves only options and appends NOCONTENT to the wire separately. The fallback therefore believes content is present and consumes alternating IDs as values, losing results when multiple documents match; preserve an effective options object containing NOCONTENT: true as well as sending the token.
Useful? React with 👍 / 👎.
| if (options?.PAYLOAD) { | ||
| parser.push('PAYLOAD', options.PAYLOAD); |
There was a problem hiding this comment.
When a custom scorer deliberately uses an empty-string payload, this truthiness check omits both PAYLOAD and its value even though the public RedisArgument type permits an empty string; an empty Buffer is sent, so equivalent binary-safe inputs also behave inconsistently. Check for undefined instead so an explicitly supplied empty payload reaches the scorer.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5fde9ebd4
ℹ️ 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".
| parser.push('FT.SEARCH', index, query); | ||
|
|
||
| parseSearchOptions(parser, options); | ||
| parser.preserve = options; |
There was a problem hiding this comment.
When a caller mutates or reuses the options object after invoking ft.search but before its promise settles, parser.preserve still references that same object. The wire arguments have already been serialized from the original flags, while the reply transformer later reads the mutated flags, so removing WITHSCORES (or changing NOCONTENT, payload, sort-key, or RETURN settings) causes the RESP2 response to be parsed with the wrong layout. Preserve an immutable snapshot of the layout-relevant options instead.
Useful? React with 👍 / 👎.
| return [String(summary), Array.isArray(children) ? children.map(normalizeScoreExplain) : []]; | ||
| } | ||
| return String(raw); |
There was a problem hiding this comment.
When EXPLAINSCORE is used with withTypeMapping({ [RESP_TYPES.BLOB_STRING]: Buffer }), the decoder supplies explanation summaries and leaves as Buffers, but these String conversions turn every one back into text in both RESP2 and RESP3. The new scoreExplain field therefore ignores the caller's configured blob mapping; its recursive type and normalization should retain the mapped blob-string values.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Reviewed by Cursor Bugbot for commit f5fde9e. Configure here.
| parser.push('FT.SEARCH', index, query); | ||
|
|
||
| parseSearchOptions(parser, options); | ||
| parser.preserve = options; |
There was a problem hiding this comment.
Profile search drops reply layout options
Medium Severity
The RESP2 transformer now sizes each document from parser.preserve (WITHSCORES, EXPLAINSCORE, NOCONTENT, WITHPAYLOADS, WITHSORTKEYS, empty RETURN), but parseSearchOptions never stores those options. PROFILE_SEARCH still emits the new flags and then calls this transformer with preserve unset, so RESP2 profile hits misread scores as ids and skip or merge later documents.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f5fde9e. Configure here.
| ...(score !== undefined && !isNaN(score) ? {score} : {}), | ||
| ...(scoreExplain !== undefined ? {scoreExplain} : {}), | ||
| ...(payload !== undefined ? {payload} : {}), | ||
| ...(sortKey !== undefined ? {sortKey} : {}), |
There was a problem hiding this comment.
Null payloads differ between protocol versions
Low Severity
RESP2 copies payload and sortKey whenever the slot is not undefined, so a Redis null placeholder becomes null on the document. RESP3 drops null and omits the property. The same missing payload is therefore null on RESP2 and absent on RESP3, which does not match the optional string | Buffer type or the WITHPAYLOADS test that expects undefined.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f5fde9e. Configure here.
2 participants
Description
This PR resolves issue #2143 by adding full support for the
WITHSCORESoption inFT.SEARCHcommands within@redis/search.Previously, the
WITHSCORESflag was either ignored during argument parsing or its returned scores were dropped during reply transformation, causing the relevance score to be missing from search results.This change ensures that:
WITHSCORESis properly serialized into the command arguments when requested.scoreis correctly parsed and extracted in both RESP2 and RESP3 reply transformers.SearchReplytype definition is updated to expose an optionalscore?: numberproperty on each returned document.Checklist
npm testpass with this change (including linting)?Note
Medium Risk
Reply parsing is option-driven and replaces prior RESP2 shape heuristics; incorrect
preserveor field order could mis-parse scores or document fields for existing callers.Overview
Extends
FT.SEARCHin@redis/searchso common RediSearch flags are serialized and reflected in typed results—not onlyWITHSCORES, but alsoEXPLAINSCORE,NOCONTENT,WITHPAYLOADS,WITHSORTKEYS,FILTER(including±Infinity→-inf/+inf),GEOFILTER, andPAYLOAD.Reply handling now depends on
parser.preserve: RESP2 walks each hit using the requested options (scores, explain trees, payloads, sort keys, emptyvaluewhenNOCONTENTorRETURN: []); RESP3 mapsscore,payload, andsortkeyonto each document.SearchReplydocuments gain optionalscore,scoreExplain,payload, andsortKey.searchNoContentreusesSEARCH.parseCommand, always appendsNOCONTENT, preserves options for transforms, and restricts options that conflict with ID-only replies. Tests cover argument building,preserve, and live Redis behavior (includingNOCONTENTvsRETURNand blob sort keys).Reviewed by Cursor Bugbot for commit f5fde9e. Bugbot is set up for automated code reviews on this repo. Configure here.