Skip to content

DOC-6968 Complete cmds_generic, and make C examples able to fail - #3812

Open
andy-stark-redis wants to merge 5 commits into
mainfrom
DOC-6968-cmds-generic-scan-examples
Open

DOC-6968 Complete cmds_generic, and make C examples able to fail#3812
andy-stark-redis wants to merge 5 commits into
mainfrom
DOC-6968-cmds-generic-scan-examples

Conversation

@andy-stark-redis

@andy-stark-redis andy-stark-redis commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

DOC-6968 — the four SCAN steps for seven clients

Adds scan1scan4 to hiredis, go-redis, jedis, lettuce-async, lettuce-reactive, ioredis and predis in the cmds_generic set. 27 implementations, 822 lines.

Omission warnings: 63 → 36. content/commands/scan.md is back to 12 client tabs on scan1, scan2 and scan4 (from 5), and 12 on scan3 where PHP is legitimately absent — see below.

@dwdougherty this is the "next PR" from #3811: the warnings that triggered your OCD are now down by 43%, and scan.md was the page paying the visible price.

18 is not the number of matching keys

*11* matches 19 of key:1key:1000 — I counted them. The reference gets 18 because its COUNT 1000 call continues from the cursor left by four preceding default-COUNT iterations, one of which has already yielded a match. My first ioredis attempt restarted at cursor 0, got 19, and failed its own assertion, which is how this surfaced.

Every tab now mirrors the reference's continuation semantics. One nuance for the record: on Redis 8.8 the observed split is 0, 0, 0, 1, 18, whereas the page's CLI transcript shows the match landing on the first iteration. Same total, different distribution — the split depends on iteration order, which matters for the next point.

Why Go's scan2 prints less than its siblings

A Go Example function only runs under go test if it has an // Output: block, and that block must match stdout exactly. Encoding the per-iteration counts (0,0,0,1) would make the Go test fail whenever SCAN's iteration order shifted — a red build for a reason unrelated to the docs.

So Go runs the four iterations but prints only the final 18. Same headline number, less visible narrative, no new fragility. The alternative (match the siblings exactly, accept Go as the canary that breaks first) was considered and rejected.

predis cannot express scan3, so PHP is absent from it

SCAN.php::prepareOptions in predis 3.5.1 emits only MATCH and COUNT. TYPE is silently dropped — probed against a live server:

with TYPE=zset -> geokey, stringkey, zkey
without TYPE   -> geokey, stringkey, zkey

stringkey is a plain string. An idiomatic-looking $r->scan(0, ['TYPE' => 'zset']) returns everything, which is worse than an absent tab, and using executeRaw in a docs example teaches the wrong thing. PHP therefore has scan1, scan2 and scan4 only.

This is the first real use of the behaviour added in #3811: PHP's absence renders as a cleanly omitted tab rather than a 46-line whole-file dump, and the one remaining warning for cmds_generic/scan3 is a truthful record rather than noise.

Verification

All seven PASS against Redis 8.8.0 via build/example-test-harness/run.sh cmds_generic, including hiredis through the portable C runner added earlier in this ticket. Observed output was checked against every >>> comment.

Two pre-existing failures in that sweep are not from this change — verified by re-running both with these changes stashed:

Client Failure Cause
nredisstack CS0246: SkipIfRedisFactAttribute could not be found the portable C# stub lacks that attribute
rust-async unresolved import futures_util, AsyncIter is not an iterator async iteration API drift in the pinned redis-rs

Neither file is touched here. Both deserve their own tickets.

What's left

36 warnings: cmds_generic 18 (del/expire/ttl 5 each, exists 2, scan3 1 for PHP), cmds_hash 12 (C and ioredis), query_vector 4 (deliberately out of scope), cmds_stream 2.

🤖 Generated with Claude Code


Note

Low Risk
Changes are limited to documentation examples, test harness behavior, and author guidance—no production services or runtime APIs.

Overview
Finishes cmds_generic doc examples: scan1–scan4 across seven clients, plus del, exists, expire, and ttl where they were missing. Command pages regain full client tabs; predis still omits scan3 because SCAN … TYPE is not supported in the client API.

SCAN scan2 examples use cursor continuation and expect 18 keys for *11* after a high-COUNT pass, matching reference semantics (not a fresh cursor at 0).

hiredis C examples now use a CHECK_REPLY guard (in REMOVE blocks) after every redisCommand, with failed value checks return 1, so the harness can fail on error replies—not only on exit 0. Fixes SET mykey %s for values with spaces (no CLI-style quoting). The pattern is documented in HIREDIS_TEST_PATTERNS.md.

cmds_hash gains hiredis steps and ioredis hash command snippets; cmds_string and landing.c get the same reply checking.

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


Second commit — del, exists, expire, ttl: cmds_generic is now complete

Adds the remaining 17 steps across hiredis, ioredis, lettuce-async, lettuce-reactive and predis.

Omission warnings site-wide: 36 → 19. commands/del.md, exists.md, expire.md and ttl.md are all back to 12 client tabs. The only cmds_generic warning left is PHP's scan3, which predis cannot express (see above).

Remaining 19: cmds_hash 12, query_vector 4 (out of scope by decision), cmds_stream 2, cmds_generic/scan3 1.

A bug in the C example, and a worse gap behind it

The bug. redisCommand(c, "SET mykey \"Hello World\"") does not work — hiredis splits its format string on whitespace and doesn't honour CLI-style quoting, so that became four tokens and the server replied ERR syntax error. Binding the value as an argument ("SET mykey %s", "Hello World") fixes it, and the expiry sequence then comes out correctly: OK, 1, 10, OK, -1, 0, -1, 1, 10.

The gap that let it through. The harness reported PASS the entire time. C examples' assertions only printf("ASSERTION FAILED") — they never touch the exit code — and hiredis doesn't abort on an error reply. So for C, a green run has only ever meant "the binary exited 0". This failure even left the final TTL at 10 by coincidence, so the value check would have passed anyway. I found it by reading the output.

The 13 assertions in that file now return 1 as well as printing. Proven, not assumed: changing one expected value from 2 to 999 turns the sweep red; restoring it turns it green.

Being explicit about what this does not fix: it would not have caught today's bug. A mid-example error reply still slips through, because nothing checks reply->type == REDIS_REPLY_ERROR. Catching that class needs a guard after each command across every C example — worth doing deliberately rather than bolting on here.

Verification

Every client in cmds_generic passes against Redis 8.8.0 except the two pre-existing, unrelated failures already documented on the first commit (nredisstack's missing SkipIfRedisFact stub; rust-async's futures_util/AsyncIter drift). Neither file is touched by this PR.

Per-client output was checked line by line against every >>> comment, including the expire NX/XX sequence, where XX must be a no-op on a key whose expiry SET has just cleared.


Third commit — make C examples able to fail at all

The bug in the previous commit exposed something bigger than the bug: a green harness run has never told you anything about a C example. There's no assertion framework, hiredis returns an error reply rather than failing the call, and it doesn't abort — so an example can send a malformed command, print a wrong value, and exit 0.

This adds a CHECK_REPLY guard after all 62 redisCommand calls across the four C examples, plus the macro behind it — everything inside REMOVE blocks, so published snippets stay plain redisCommand:

reply = redisCommand(c, "SET mykey %s", "Hello World");
// REMOVE_START
CHECK_REPLY(reply);
// REMOVE_END

Proven on the hardest case, not the convenient one

Reintroducing an error reply that gets printed with %lld — which prints 0, never the error text — now fails the sweep:

REDIS ERROR: ERR wrong number of arguments for 'incr' command
  hiredis            FAIL

Restoring the file returns it to PASS.

That case is why I rejected the cheap version. I'd floated scanning hiredis's stdout for ERR in run_hiredis — one line, covers every C example forever. It's the wrong call: it misses the %lld case entirely (the error never reaches stdout), and it would false-positive on any future example that deliberately demonstrates an error reply. The guard has to sit at the call site.

Also

  • Two unassigned cleanup DELs in cmds_hash.c could be neither checked nor freed. Now assigned, guarded and freed — both in REMOVE blocks, so the page is unchanged.
  • Value assertions now return 1 as well as printing, in the file where that was added earlier in this branch.
  • The rule is written into assets/hiredis/HIREDIS_TEST_PATTERNS.md in the tce-examples skill, with the reasoning and the quoting trap, because the guard is only as good as the next author remembering it.

Verification

All four files compile clean under -Wall. hiredis passes cmds_generic, cmds_hash and cmds_string.

landing.c is compile-checked only — the harness reports SKIP (no source) for the landing set, because that file lives at local_examples/client-specific/c/ rather than the conventional local_examples/<set>/<client>/. Its guard is therefore unexercised, which is worth knowing rather than assuming.

Adds scan1, scan2, scan3 and scan4 to the hiredis, go-redis, jedis,
lettuce-async, lettuce-reactive, ioredis and predis files, clearing 27 of the
63 omission warnings the shortcode fix surfaced. content/commands/scan.md goes
back to twelve client tabs on scan1, scan2 and scan4.

The 18 in scan2 is not the number of matching keys. `*11*` matches 19 of
key:1..key:1000; the reference gets 18 because its COUNT 1000 call continues
from the cursor left by four preceding default-COUNT iterations, one of which
has already yielded a match. My first attempt restarted at cursor 0, got 19 and
failed its own assertion, which is how this surfaced. Every tab now mirrors the
reference's continuation semantics. Worth knowing that the observed split on
Redis 8.8 is 0,0,0,1,18 whereas the CLI transcript on the page shows the match
landing on the first iteration instead — same total, different distribution,
because the split depends on iteration order.

That order-dependence is why Go's scan2 differs from its siblings. A Go Example
function only runs under `go test` if it has an `// Output:` block, and that
block must match stdout exactly, so printing per-iteration counts would make
the test fail whenever SCAN's iteration order shifts — a red build for a reason
unrelated to the docs. Go therefore runs the four iterations but prints only
the final 18. Same headline number, less visible narrative, no new fragility.

predis cannot express scan3 at all, so PHP is deliberately absent from that
step. `SCAN.php::prepareOptions` in predis 3.5.1 emits only MATCH and COUNT, so
`$r->scan(0, ['TYPE' => 'zset'])` silently drops the filter: probed against a
live server it returned a plain string key alongside the two sorted sets, byte
for byte identical to the unfiltered call. An idiomatic-looking call that
quietly returns everything is worse than an absent tab, and the omission now
renders as a missing tab rather than a whole-file dump — the first real use of
the behaviour added earlier in this ticket.

All seven pass against Redis 8.8.0. Two clients fail cmds_generic for
pre-existing, unrelated reasons, verified by re-running with these changes
stashed: nredisstack cannot compile because the portable C# stub lacks
SkipIfRedisFactAttribute, and rust-async fails on `unresolved import
futures_util` plus AsyncIter no longer being an iterator in the pinned crate.
Neither file is touched here.

Learned: scan2's expected 18 is a cursor-continuation artifact, not a match count — a new tab must continue from the cursor the earlier iterations returned, never restart at 0, or it correctly gets 19 and disagrees with every other tab
Constraint: Go example funcs only execute with an exact `// Output:` block, so scan2 in Go must not print per-iteration counts — 0,0,0,1 is iteration-order dependent and would break on an unrelated Redis change
Constraint: predis 3.5.1 SCAN accepts only MATCH and COUNT; TYPE is silently dropped and every key comes back, which is why PHP has no scan3 example
Directive: do not "complete" PHP's scan3 with $r->scan(0, ['TYPE' => 'zset']) — it looks idiomatic, filters nothing, and was verified wrong against a live server
Gaps: nredisstack and rust-async already fail cmds_generic for unrelated reasons (missing SkipIfRedisFact stub; futures_util/AsyncIter drift in the pinned redis-rs) — do not read those as regressions from this change
Ticket: DOC-6968
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

DOC-6968

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

🧠 Redis Memory

Found 10 related items from repository history (1 new this commit):

Memory updated at 833febc

…c clients

Adds the 17 missing steps across hiredis, ioredis, lettuce-async,
lettuce-reactive and predis, taking cmds_generic to full coverage. Omission
warnings site-wide fall from 36 to 19, and commands/del.md, exists.md,
expire.md and ttl.md are all back to twelve client tabs. The only cmds_generic
warning left is PHP's scan3, which predis cannot express at all.

Two problems in the C example, and the second matters more than the first.

The bug: `redisCommand(c, "SET mykey \"Hello World\"")` does not work. hiredis
splits its format string on whitespace and does not honour CLI-style quoting,
so that became four tokens and the server answered ERR syntax error. The value
has to be bound as an argument — `redisCommand(c, "SET mykey %s", "Hello
World")` — and once it was, the expiry sequence came out right: OK, 1, 10, OK,
-1, 0, -1, 1, 10.

The gap: the harness reported PASS the whole time. The C examples' assertions
only printf "ASSERTION FAILED"; they never affect the exit code, and hiredis
does not abort on an error reply, so for C a green run has only ever meant "the
binary exited 0". This particular failure even left the final TTL at 10 by
coincidence, so the value check would have passed regardless. I found it by
reading the output, not from the harness.

So the 13 assertions in this file now `return 1` as well as printing. Proven
rather than assumed: deliberately changing one expected value from 2 to 999
turns the sweep red, and restoring it turns it green. Worth being clear that
this would NOT have caught today's bug — an error reply mid-example still slips
through, because nothing checks reply->type for REDIS_REPLY_ERROR. Catching
that class needs a guard after each command, across every C example, and is
better done deliberately than bolted on here.

nredisstack and rust-async still fail cmds_generic for the pre-existing,
unrelated reasons recorded on the previous commit; neither file is touched.

Learned: for C examples a harness PASS meant only that the binary exited 0 — the assertions printed and returned nothing — so an example could emit ERR syntax error mid-run and still be reported green
Constraint: hiredis does not honour CLI-style quoting in its format string; a value containing spaces must be bound with %s or it is split into separate arguments and the command fails
Constraint: keep the `return 1` alongside each ASSERTION FAILED printf in this file, or the harness stops being able to fail on a wrong value
Gaps: nothing checks reply->type == REDIS_REPLY_ERROR in the C examples, so a mid-example error reply still passes; a guard after each command, applied across all C examples, is the fix
Ticket: DOC-6968
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@andy-stark-redis andy-stark-redis changed the title DOC-6968 Add the four SCAN steps to seven cmds_generic client examples DOC-6968 Complete the cmds_generic example set across all clients Aug 14, 2026
Adds a CHECK_REPLY guard after all 62 redisCommand calls across the four C
examples, plus the macro that backs it, all inside REMOVE blocks so the
published snippets stay plain redisCommand.

Until now a green harness run told you nothing about a C example. There is no
assertion framework, hiredis returns an error REPLY rather than failing the
call, and it does not abort — so an example could send a malformed command,
print a wrong value and still exit 0. That is not hypothetical: the previous
commit on this branch shipped `SET mykey "Hello World"` (hiredis splits its
format string on whitespace and ignores CLI quoting), the server answered ERR
syntax error, and the sweep reported PASS. I found it by reading output.

Proven on the hardest case rather than the convenient one. Reintroducing an
error reply that gets printed with %lld — which prints 0, not the error text —
now fails the sweep with "REDIS ERROR: ERR wrong number of arguments for
'incr' command". That specific case is why the cheap version was rejected:
scanning the program's stdout for ERR would have missed it entirely, and would
also false-positive on any future example that deliberately demonstrates an
error reply.

Two incidental fixes in cmds_hash.c: the two cleanup DELs were unassigned, so
they could be neither checked nor freed. They are now assigned, guarded and
freed. Both sit in REMOVE blocks, so nothing changes on the page.

The rule is written into the hiredis patterns file in the tce-examples skill,
because the guard is only as good as the next author remembering it, and a
commit cannot enforce that.

Verified: all four files compile clean under -Wall, and hiredis passes
cmds_generic, cmds_hash and cmds_string. landing.c is compile-checked only —
the harness reports SKIP (no source) for the landing set because that file
lives at local_examples/client-specific/c/ instead of the conventional
local_examples/<set>/<client>/ path.

Learned: an output-scanning check for C errors is not equivalent to a call-site guard — printf("%lld", reply->integer) on an error reply prints 0, so the error never reaches stdout to be scanned for
Constraint: every redisCommand in a C example needs CHECK_REPLY in a REMOVE block, and value assertions must return 1 as well as printing; without both, a C example can print wrong values and still exit 0
Rejected: scanning hiredis stdout for "ERR" in run_hiredis as the primary check | incomplete for the %lld case, and it would false-positive on a future example that deliberately shows an error reply
Gaps: landing.c cannot be executed by the harness (SKIP: no source) because it sits outside the local_examples/<set>/<client>/ convention, so its guard is compile-checked only
Ticket: DOC-6968
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@andy-stark-redis andy-stark-redis changed the title DOC-6968 Complete the cmds_generic example set across all clients DOC-6968 Complete cmds_generic, and make C examples able to fail Aug 14, 2026
Comment thread local_examples/cmds_generic/hiredis/cmds_generic.c Outdated
…uard commit

Adds the six missing steps (hset, hget, hgetall, hdel, hvals, hexpire) to the C
and ioredis cmds_hash examples, and repairs a real bug the previous commit on
this branch introduced.

Omission warnings are now 7, down from 63 when the shortcode fix first
surfaced them. All six cmds_hash command pages carry a full 15 panes. The
remaining 7 are not work items: PHP cannot express SCAN TYPE with predis,
Ruby and ioredis lack XCFGSET for xadd2, and the four query_vector panes are
out of scope by decision. That is the floor.

The bug. My CHECK_REPLY transformer wrapped every guard in its own REMOVE
block, including at call sites that were ALREADY inside one — the cleanup DELs.
build/components/example.py treats a nested remove anchor as fatal and
`return`s, abandoning the rest of the file, so every C example lost the steps
declared after its first cleanup command: cmds_generic C fell from 12 named
steps to 2, cmds_string to 2, cmds_hash to 4.

What makes that worse than a broken build is what it would have done if merged.
Those steps still exist in the source files, so the shortcode change earlier in
this ticket would have read C's named_steps as non-empty but missing the step,
and quietly omitted the C tab from roughly twenty command pages. Silent
coverage loss, which is the exact failure the guard was written to prevent, one
layer up.

Twelve guards are now emitted bare, without their own markers, where the call
already sits inside a REMOVE block. C is back to 12, 11 and 5 named steps
respectively, and 19 C panes render site-wide.

Two reasons I did not catch it immediately, both worth avoiding next time. The
harness stayed green throughout, because nested markers are a docs-parser
concern and the compiled binary neither knows nor cares. And I ran make.py with
its output piped through `grep -iE "error" | head -8`, where eight GitHub
rate-limit warnings filled the window before "ERROR:root:Nested remove anchor"
could appear. The regression showed up only as an unexplained warning count of
24 where 7 was expected — I nearly rationalised it before measuring.

Verified after the fix: zero nested-anchor errors from make.py, all four C files
compile clean under -Wall, hiredis passes cmds_generic, cmds_hash and
cmds_string, the full cmds_hash sweep is green for all 13 testable clients, and
deliberately corrupting a command still fails the sweep so the guard itself
still works.

Learned: build/components/example.py ABORTS a whole example file on a nested REMOVE anchor, silently dropping every step after it — a mechanical marker edit can therefore delete a client's coverage without failing the harness, which never parses markers
Constraint: a CHECK_REPLY guard at a call site already inside a REMOVE block must be emitted bare; wrapping it in its own REMOVE_START/REMOVE_END nests the anchors and kills the file
Learned: never read make.py's diagnostics through `head` — GitHub rate-limit warnings crowd out real ERROR lines, and this one was invisible for exactly that reason
Ticket: DOC-6968
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@andy-stark-redis andy-stark-redis self-assigned this Aug 14, 2026
@andy-stark-redis andy-stark-redis added the clients Client library docs label Aug 14, 2026

@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 using high effort 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 589cc53. Configure here.

Comment thread local_examples/cmds_generic/go-redis/cmds_generic_test.go
Redis does not promise a field order for HSCAN, HSCAN NOVALUES or HVALS, and
four of the examples added on this branch depended on it. Found by Cursor
Bugbot, which also spotted that Jedis and Lettuce already sort for the same
step, so the unsorted tabs could flake while their siblings stayed green.

Fixed by removing the dependency rather than by sorting a flat pair list, which
would interleave fields with the wrong values:

- go-redis scan4 collects the interleaved HSCAN reply into a map and prints
  that, because fmt prints map keys in sorted order regardless of arrival
  order. Its NOVALUES list is sorted. This one mattered most: a Go Example
  function compares stdout against its `// Output:` block exactly, so a shifted
  order is a hard failure.
- ioredis scan4 pairs the reply into an object instead of asserting on element
  positions; its NOVALUES list is sorted.
- predis scan4 sorts the NOVALUES list. Its associative assert was already
  order-insensitive, because PHP compares assoc arrays by key, but the NOVALUES
  one is a list and so was not — a distinction I got wrong at first.
- ioredis cmds_hash hvals is sorted, since HVALS follows field order.

Proven, not argued. Setting hash-max-listpack-entries to 0 forces small hashes
onto a hashtable, where HSCAN returns fields in a genuinely different order
(observed: d 4 c 3 b 2 a 1 for a hash written a, b, c, d). All four fixed
clients pass under that config.

That experiment also turned up pre-existing fragility of exactly the same kind,
which this commit deliberately does NOT touch: redis-py, node-redis and
lettuce-reactive fail cmds_generic and cmds_hash under hashtable encoding, and
predis and ruby fail cmds_hash. Confirmed as order failures, not something else
— node-redis received [{field: b}, {field: a}] where it expected the reverse,
and ruby got ["World", "Hello"]. None of it flakes under the default
hash-max-listpack-entries of 512, because these examples use two or three
fields and so stay listpack-encoded; the realistic trigger is a future example
with a large hash, or a server with a tuned threshold. Fixing it means editing
the reference implementation every other tab is copied from, which deserves its
own review rather than riding along here.

Learned: forcing hash-max-listpack-entries to 0 is a cheap way to test whether a hash example depends on field order — it flips small hashes to hashtable encoding, where HSCAN order genuinely differs
Constraint: never sort the flat HSCAN reply to make it deterministic — fields and values are interleaved, so sorting pairs the wrong values together; collect into a map or object instead
Learned: PHP's == is order-insensitive for associative arrays but not for lists, so an assoc assert can be safe while the NOVALUES list assert beside it is not
Gaps: redis-py, node-redis, lettuce-reactive, predis and ruby hash examples still assume field order and fail under hashtable encoding; harmless at the default threshold, worth its own ticket
Ticket: DOC-6968
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@dwdougherty dwdougherty left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM.

@andy-stark-redis

Copy link
Copy Markdown
Contributor Author

Thanks @dwdougherty !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clients Client library docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants