feat(search): add WITHSCORES support to FT.SEARCH (#2143) - #3432
feat(search): add WITHSCORES support to FT.SEARCH (#2143)#3432watersRand wants to merge 1 commit 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.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Reviewed by Cursor Bugbot for commit 7741133. Configure here.
| } | ||
| documents.push({ | ||
| id: reply[i++] as string, | ||
| ...(score !== undefined ? {score} : {}), |
There was a problem hiding this comment.
RESP2 parser misreads scores and IDs
High Severity
The RESP2 transformer treats a numeric string followed by a fields array as a leading score, but default FT.SEARCH replies are id then fields, so numeric keys are consumed as scores. WITHSCORES replies are id, then score, then fields, and withoutDocuments treats the score at reply[2] as missing content, so hits lose fields and extra documents appear.
Reviewed by Cursor Bugbot for commit 7741133. Configure here.
|
|
||
| if (options?.WITHSCORES) { | ||
| parser.push('WITHSCORES'); | ||
| } |
There was a problem hiding this comment.
NOCONTENT WITHSCORES mixes scores into IDs
Medium Severity
WITHSCORES is on shared FtSearchOptions, so searchNoContent can send it. That command's RESP2 transformer still treats every element after total as an id, so interleaved scores show up as extra document ids. RESP3 drops scores and keeps ids, so the same call disagrees across protocols.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7741133. Configure here.
1 participant
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
Low Risk
Additive search option and reply parsing with tests; no auth, persistence, or breaking type changes beyond an optional
scorefield.Overview
Adds end-to-end
WITHSCORESsupport forclient.ft.search/FT.SEARCHin@redis/search.When
{ WITHSCORES: true }is passed, the command builder now emits theWITHSCORESargument (alongside the defaultDIALECT). Reply transformers for RESP2 and RESP3 parse relevance scores and attach an optionalscoreon each document inSearchReply; without the flag, behavior stays the same.Coverage includes argument-serialization tests and an integration test that asserts
documents[0].scoreis a number when scores are requested.Reviewed by Cursor Bugbot for commit 7741133. Bugbot is set up for automated code reviews on this repo. Configure here.