Skip to content

RI-8296 Exact u64 array integer replies (per-command ioredis bigint)#6100

Merged
VaskoAtanasovRedis merged 15 commits into
mainfrom
be/RI-8296/ioredis-smart-int-parse
Jul 2, 2026
Merged

RI-8296 Exact u64 array integer replies (per-command ioredis bigint)#6100
VaskoAtanasovRedis merged 15 commits into
mainfrom
be/RI-8296/ioredis-smart-int-parse

Conversation

@VaskoAtanasovRedis

@VaskoAtanasovRedis VaskoAtanasovRedis commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

What

Makes array u64 integer replies exact. RESP encodes : integers as signed
i64, so Redis sends u64 values ≥ 2^63 as bulk strings (exact) but values in
(2^53, 2^63) as RESP integers — which ioredis coerces to a JS number and
rounds above 2^53. That zone was silently lossy for array indexes/lengths.

A patched ioredis DataHandler toggles the parser's stringNumbers per
command via the command queue and returns BigInt only for commands that
opt in with { integerReply: 'bigint' }. All other commands are untouched.

  • The u64 integer-reply array commands opt in via the shared
    ARRAY_U64_INTEGER_REPLY_COMMANDS set: ARLEN, ARCOUNT, ARNEXT,
    ARSCAN, ARGREP, ARINSERT, ARRING, AROP, ARINFO.
  • CLI and Workbench detect those commands and opt in too; reply formatters
    stringify BigInt.
  • The browser array service opts in per command as well: reads serialize
    index/length via toIndexString, and aggregate (AROP) requests bigint so
    AND/OR/XOR/MATCH/USED results stay exact — no BigInt reaches
    res.json, responses remain decimal strings.
  • ArrayKeyInfoStrategy issues ARLEN/ARCOUNT with the bigint opt-in rather
    than pipelining them (a pipeline can't carry per-command options).
  • node-redis is not handled: ioredis is the default client, and node-redis is
    an experimental env-only strategy with no production caller, so it's left
    untouched (bigint there would need RESP3 type mapping — deferred).

Testing

  • Unit specs assert the { integerReply: 'bigint' } opt-in on the array
    service calls and on the CLI path (including AROP and ARINFO).
  • Integration tests exercise the (2^53, 2^63) gap zone for ARLEN + ARSCAN,
    the >2^53 ARGREP search assertion, and an AROP OR aggregate over a
    gap-zone value (run on the 8.8 RTE).
  • A raw-socket test locks the wire contract: a gap-zone length arrives as a
    RESP integer (:), a value ≥ 2^63 as a bulk string ($).

CLI

image

Workbench

image

Closes #RI-8296


Note

Medium Risk
Touches the patched ioredis transport and all array integer paths; incorrect opt-in or patch application could affect non-array commands or CI if patches are not applied at install time.

Overview
Fixes silent precision loss for Redis array u64 integers in (2^53, 2^63) when ioredis maps RESP integers to JS number.

Adds a patch-package change to ioredis so commands can opt in with { integerReply: 'bigint' }; the parser toggles stringNumbers per queued command and converts integer replies to BigInt. The Redis client layer exposes integerReply on command options and includes bigint in reply types.

Introduces ARRAY_U64_INTEGER_REPLY_COMMANDS and wires the bigint opt-in through the browser array service (scan, search, length/count/next, aggregate), CLI, and Workbench. ArrayKeyInfoStrategy fetches ARLEN/ARCOUNT outside the pipeline so per-command options apply. Reply formatters (API + CLI) tag bigint as { type: 'integer', value: '...' }; the UI cliTextFormatter renders those as (integer) N.

Integration and raw-socket tests assert exact strings above MAX_SAFE_INTEGER; the test Docker image copies patches before yarn install so the ioredis patch is applied.

Reviewed by Cursor Bugbot for commit 53be495. Bugbot is set up for automated code reviews on this repo. Configure here.

@jit-ci

jit-ci Bot commented Jun 23, 2026

Copy link
Copy Markdown

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

Comment thread redisinsight/api/patches/ioredis+5.3.2.patch

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 12a8e30808

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Code Coverage - Backend unit tests

St.
Category Percentage Covered / Total
🟢 Statements 93.06% 16355/17575
🟡 Branches 75.28% 5185/6888
🟢 Functions 87.37% 2518/2882
🟢 Lines 92.91% 15633/16826

Test suite run success

3633 tests passing in 317 suites.

Report generated by 🧪jest coverage report action from 481ddc7

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c4390b2. Configure here.

Comment thread redisinsight/api/patches/ioredis+5.3.2.patch
@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Code Coverage - Integration Tests

Status Category Percentage Covered / Total
🟡 Statements 79.06% 18077/22863
🟡 Branches 61.9% 8420/13602
🟡 Functions 66.83% 2452/3669
🟡 Lines 78.67% 17014/21627

@VaskoAtanasovRedis VaskoAtanasovRedis force-pushed the be/RI-8296/ioredis-smart-int-parse branch from ce6b4aa to 413f584 Compare June 23, 2026 10:48

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 413f5849a7

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread redisinsight/api/patches/ioredis+5.3.2.patch
@VaskoAtanasovRedis VaskoAtanasovRedis force-pushed the be/RI-8296/ioredis-smart-int-parse branch 3 times, most recently from e333211 to 3a11af7 Compare June 23, 2026 12:07

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3a11af787d

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@VaskoAtanasovRedis VaskoAtanasovRedis force-pushed the be/RI-8291/argrep-search-endpoint branch from 96c3d38 to c73ec0d Compare June 23, 2026 12:21
@VaskoAtanasovRedis VaskoAtanasovRedis force-pushed the be/RI-8296/ioredis-smart-int-parse branch from 3a11af7 to 5e323a9 Compare June 23, 2026 12:26
Base automatically changed from be/RI-8291/argrep-search-endpoint to main June 23, 2026 13:10

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4300179f7d

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@VaskoAtanasovRedis VaskoAtanasovRedis force-pushed the be/RI-8296/ioredis-smart-int-parse branch from 4300179 to cd76ac1 Compare June 30, 2026 10:01

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cd76ac18db

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7db498cd04

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread redisinsight/api/src/modules/redis/client/node-redis/node-redis.client.ts Outdated
@VaskoAtanasovRedis VaskoAtanasovRedis force-pushed the be/RI-8296/ioredis-smart-int-parse branch from 7db498c to 8850b1f Compare June 30, 2026 13:10

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8850b1f558

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread redisinsight/ui/src/utils/transformers/cliTextFormatter.ts
…gint [RI-8296]

Adopt the ioredis patch that toggles the parser's stringNumbers per command (via the command queue) and returns BigInt for commands that opt in with { integerReply: 'bigint' } — so other commands are untouched. Opt in the five array integer-reply commands (ARLEN/ARCOUNT/ARNEXT/ARSCAN/ARGREP); their indexes/counts are normalised to strings by toRequiredIndexString. Replaces the earlier global redis-parser approach.

References: #RI-8296
…296]

Detect array u64 integer-reply commands (ARLEN/ARCOUNT/ARNEXT/ARSCAN/ARGREP) in the CLI and request { integerReply: 'bigint' }; the raw/utf-8/text output formatters stringify BigInt so the result isn't rounded and doesn't break JSON serialization.

References: #RI-8296
…set [RI-8296]

Workbench executor opts array u64 integer-reply commands into { integerReply: 'bigint' } and its UTF8/ASCII reply transformers stringify BigInt — same as the CLI. Extract ARRAY_U64_INTEGER_REPLY_COMMANDS into browser-tool-commands.ts as one source of truth for both.

References: #RI-8296
…8296]

With the per-command bigint engine, the search endpoint returns the exact u64 index, so the >2^53 round-trip (de-scoped on #6090) is proven again.

References: #RI-8296
…8296]

main's read-endpoint precision tests use values >= 2^63, which Redis sends as RESP bulk strings (exact without the engine). Add round-trip cases at 2^53+1 — a RESP integer that a JS number rounds — so ARLEN and ARSCAN actually exercise the per-command bigint path. ARCOUNT/ARNEXT can't reach the zone (count/cursor never approach 2^53).

References: #RI-8296
Raw-socket probe asserting the actual RESP type byte: a length in (2^53, 2^63) comes back as a RESP integer (':') exact on the wire, and a length >= 2^63 as a bulk string ('$'). Pins the boundary the per-command bigint engine relies on, so a future Redis encoding change trips a test instead of invalidating a comment. Gated to standalone 8.8.

References: #RI-8296
…-8296]

Use requirements('rte.type<>CLUSTER') instead of an in-test this.skip(); the imperative form tripped the strict check tsconfig (TS2683 'this' implicitly any).

References: #RI-8296
Key-info read ARLEN/ARCOUNT via sendPipeline, which can't carry per-command options, so length/count rounded in the (2^53, 2^63) zone. Read them per-command with the bigint opt-in (TTL/MEMORY USAGE stay pipelined for graceful degradation); add a real-server gap-zone integration case.

The CLI TEXT formatter recursed array leaves through JSON.stringify, which throws on BigInt, so ARSCAN/ARGREP replies became failed executions. Format BigInt leaves as '(integer) N' like the top level.

References: #RI-8296
test.Dockerfile copied patches/ after yarn install, so patch-package found nothing and the ioredis bigint parser never applied — every (2^53, 2^63) array reply rounded in CI. Copy patches/ before install, mirroring the root Dockerfile.

Also fix the RESP wire test's cluster gate: conditionalIgnore has no '<>' operator (always skips), so use the supported '!rte.type=CLUSTER'.

References: #RI-8296
…RI-8296]

The RAW, ASCII and UTF8 reply formatters stringified bigint integer replies, so the CLI and Workbench rendered u64 array indexes as quoted strings instead of (integer) N. Tag them as { type: 'integer', value } and render the tag in cliTextFormatter, matching redis-cli. Add ARINSERT and ARRING to the u64 integer-reply opt-in.
AROP AND/OR/XOR return a u64 bitwise result that can exceed 2^53; without
the per-command bigint opt-in the CLI and Workbench rounded it, breaking
the exact-precision path the aggregate response documents.

References: #RI-8296
…t [RI-8296]

The added bigint test cases reused the untyped `let strategy`, so each new
`strategy.format` call raised TS7005 above the recorded baseline (masked
locally by a stale incremental tsbuildinfo; CI compiles fresh and fails).
Annotate the variable in each spec and refresh the API baseline downward.

References: #RI-8296
…-8296]

ArrayService.aggregate sent AROP without the bigint opt-in, so AND/OR/XOR
(and the MATCH/USED counts) above 2^53 rounded before toIndexString saw
them — the Array-tab API returned a rounded value while only Workbench was
exact. Request AROP with { integerReply: 'bigint' } so every operation
stays exact (SUM/MIN/MAX are bulk strings, unaffected), drop the
Workbench-only caveat from the response contract, and add a real-server
gap-zone OR test alongside the SUM one.

References: #RI-8296
ARINFO returns array metadata (length/count/cursor) that for a sparse
array can exceed 2^53, but CLI/Workbench left integerReply unset for it,
so ioredis rounded the nested integers. Add it to the opt-in set so the
metadata renders exact.

References: #RI-8296
@VaskoAtanasovRedis VaskoAtanasovRedis force-pushed the be/RI-8296/ioredis-smart-int-parse branch from c6e32ed to 481ddc7 Compare July 1, 2026 08:13
ArtemHoruzhenko
ArtemHoruzhenko previously approved these changes Jul 2, 2026
valkirilov
valkirilov previously approved these changes Jul 2, 2026
// aggregates) and must stay exact above 2^53 — callers request them with
// { integerReply: 'bigint' }.
export const ARRAY_U64_INTEGER_REPLY_COMMANDS = new Set<string>([
BrowserToolArrayCommands.ArLen,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ignore this comment: I believe this follows the names of the original Redis command, but I can't express how I feel when someone saves a few letters to produce ArLen instead of ArrayLength (especially when the other commands below are fully written Next/Scan/Grep/Insert/etc 😀

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does indeed follow the name of the command - https://redis.io/docs/latest/commands/arlen/

"src/common/transformers/redis-reply/strategies/ascii-formatter.strategey.spec.ts": {
"TS7005": 8,
"TS7034": 2
"TS7005": 1,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kudos 🍕

…Reply [RI-8296]

Replace the any params on the mockCounts helper with the real sendCommand
reply type, which already covers the number/bigint/string replies the tests
feed it.

References: #RI-8296
@VaskoAtanasovRedis VaskoAtanasovRedis merged commit a90fa20 into main Jul 2, 2026
25 checks passed
@VaskoAtanasovRedis VaskoAtanasovRedis deleted the be/RI-8296/ioredis-smart-int-parse branch July 2, 2026 07:46
KrumTy added a commit that referenced this pull request Jul 2, 2026
…build (#6149)

The E2E "Build Docker" job started failing (patch-package: "Failed to apply
patch for package ioredis") after RI-8296 (#6100) added
patches/ioredis+5.3.2.patch. The patch modified ioredis' built/*.d.ts files.

.github/build/build_modules.sh installs the API (patch applies), then runs
`yarn autoclean` with .yarnclean.prod — whose `*.ts` rule also deletes
`*.d.ts` — and then re-installs, which re-runs patch-package. Because the
patch's target .d.ts files were stripped by autoclean, patch-package can no
longer apply it and the build fails. (The host workspace build passes because
it never autocleans.)

The .d.ts changes are type-only and unused by our build: prepareCommandOptions
returns `any`, and RedisClient defines its own `integerReply` type — nothing
type-checks against ioredis' CommandOptions. So drop the two .d.ts hunks and
keep only the runtime Command.js / DataHandler.js changes. patch-package is
then idempotent across the autoclean+reinstall cycle.

Reproduced the full install -> autoclean -> reinstall sequence locally: the
.js-only patch applies cleanly on both passes (`ioredis@5.3.2 ✔`).

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants