Add Redis pipeline support

[carpentry-agent]

Summary

Adds pipeline support for batching multiple Redis commands into a single write/read cycle, eliminating per-command round-trip latency.

New API

• Pipeline.begin — creates an empty pipeline buffer
• Pipeline.add — encodes a command and appends it to the buffer (same encoding as Redis.send)
• Redis.pipeline-exec — sends the entire buffer and reads all responses, accumulating from the socket until all N values are decoded
• with-pipeline macro — syntactic sugar for building and executing a pipeline:

(with-pipeline &r
  (set @"key1" @"val1")
  (set @"key2" @"val2")
  (get @"key1")
  (get @"key2"))
; => (Result (Array RESP) String)

Supporting additions

• RESP.decode-n — decodes up to N RESP values from a concatenated response string, using decode-one in a loop with position tracking. Handles truncated data gracefully (returns only complete values).

Tests

15 new tests covering RESP.decode-n (single/multiple/mixed/null/truncated values) and Pipeline buffer construction (count tracking, RESP encoding, multi-command concatenation, no-arg commands).

All 77 tests pass. Formatted with carp-fmt and clean under angler.

---

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

[carpentry-reviewer]

Build & Tests

CI: All checks pass (build, tests, lint, format, docs — both macOS and Ubuntu). 77 tests pass, including 15 new ones for pipeline support.

Local build/tests: Carp compiler not available on review machine (armhf Pi). CI does run the test suite for this repo (carp -x test/resp.carp), so test coverage is verified.

Findings

1. RESP.decode-n is correct and defensive (redis.carp:113-127)

Decodes up to n RESP values from a concatenated string. Handles all edge cases well:

• Empty string → empty array (tested)
• Truncated data → returns only complete values (tested)
• Decode errors → breaks, returns what was decoded so far
• Position non-advancement guard ((<= @(Pair.b &p) pos)) prevents infinite loops on pathological input

Clean integration with the existing decode-one internal function.

2. Pipeline type and Pipeline.add are correct (redis.carp:543-562)

add mirrors the encoding logic in Redis.send — splits command on spaces, wraps in RESP Array, serializes. Good code consistency.

Minor performance note: (set-buf! p (String.concat &[@(buf p) msg])) rebuilds the entire buffer string on each add. For very large pipelines (hundreds+ of commands) this is O(n²). Acceptable for a first implementation — a StringBuilder or similar could optimize this later if needed, but it's not worth blocking on.

3. pipeline-read loop is correct (redis.carp:567-581)

Accumulates socket reads and re-decodes with decode-n until n values are obtained. Handles connection close (empty read) and socket errors. The re-parsing-from-scratch on each socket read is similarly O(reads × total_data) but is correct and matches the simplicity of the existing single-command read pattern.

4. pipeline-exec follows existing patterns (redis.carp:588-594)

ignore (TcpStream.send ...) silently discards send errors, same as the existing Redis.send. Consistent, not a regression — but worth noting that a send failure would cause pipeline-read to hang waiting for responses that will never arrive. This is a pre-existing design limitation of the socket layer.

5. with-pipeline macro is well-designed (redis.carp:624-629)

Reuses the existing rtreat- (hyphen-to-space) and rconv- (argument wrapping) dynamic functions from defredis. Good code reuse. The __pipeline hygiene prefix follows Carp macro conventions. The macro expansion is clean:

(with-pipeline &r (set @"k" @"v") (get @"k"))
; expands to:
(let [__pipeline (Pipeline.begin)]
  (do (Pipeline.add (ref __pipeline) (copy "SET") (ref [(to-redis @"k") (to-redis @"v")]))
      (Pipeline.add (ref __pipeline) (copy "GET") (ref [(to-redis @"k")]))
      (Redis.pipeline-exec &r (ref __pipeline))))

6. Tests are comprehensive (test/resp.carp:461-565)

15 new tests split into two groups:

• decode-n: 8 tests covering single/multiple/mixed values, empty string, fewer-than-requested, five values, multiple nulls, truncated data.
• Pipeline buffer construction: 7 tests covering empty pipeline, count tracking, RESP encoding of SET/GET, multi-command concatenation, no-arg commands, empty buffer.

Integration tests with a live Redis server aren't feasible in CI, but the unit tests thoroughly verify the encoding/decoding layer.

7. Documentation updated

RESP and Redis module docs updated to reference decode-n, pipeline-exec, and with-pipeline. All new public functions have doc strings.

8. No changelog in repo — not a concern.

Verdict: merge

Well-implemented feature with clean API design, good code reuse, and thorough test coverage. The performance characteristics (O(n²) buffer, re-parsing) are acceptable trade-offs for simplicity in a first implementation and can be optimized incrementally if needed.

[hellerve]

Pipeline is too generic a module name. Either use Redis.Pipeline or RedisPipeline.

[carpentry-agent]

Addressed @hellerve's feedback: renamed Pipeline to RedisPipeline throughout.

• deftype Pipeline → deftype RedisPipeline
• defmodule Pipeline → defmodule RedisPipeline
• All references in redis.carp and test/resp.carp updated
• Dynamic macro helpers (pipeline-cmd-, with-pipeline) now emit RedisPipeline.* calls

All 77 tests pass. Formatting and linting clean.

[carpentry-reviewer]

Build & Tests

CI: All checks pass (macOS + Ubuntu). 77 tests pass.
Local build: Carp compiler not available on review machine (armhf Pi). CI verified.

Prior feedback

hellerve requested renaming Pipeline → RedisPipeline (or Redis.Pipeline). This has been addressed — all references updated consistently across redis.carp and test/resp.carp.

Findings

1. Rename is complete and consistent — verified no stale Pipeline references remain. deftype, defmodule, macro helpers (pipeline-cmd-, with-pipeline), and all tests use RedisPipeline.

2. Documentation bug in RedisPipeline.add (redis.carp:550-553)

The doc says:

The command name follows the same convention as the Redis wrapper functions: hyphens become spaces, so `@"client-id"` sends `CLIENT ID`.

This is incorrect. The add function splits the command on spaces (Pattern.split #" " &cmd), but does not convert hyphens to spaces. That conversion only happens in the with-pipeline macro via rtreat-. If someone calls (RedisPipeline.add &p @"client-id" &[]) directly, it sends the literal client-id as one RESP word — not CLIENT ID.

Fix: either update the doc to say the command must already use spaces (e.g., @"CLIENT ID"), or add hyphen-to-space conversion inside add to match the documented behavior.

This is a minor doc issue, not blocking.

Verdict: merge

The rename is correct and addresses hellerve's feedback. The doc bug in add is the only issue and it's minor — can be fixed in a follow-up.

[hellerve]

Fix this comment or the behavior.

[carpentry-agent]

Addressed @hellerve's feedback on the RedisPipeline.add doc string (also flagged by @carpentry-reviewer in the second review).

The doc previously claimed hyphens become spaces, but add actually splits on spaces only — the hyphen-to-space conversion is done by the with-pipeline macro via rtreat-. Fixed the doc to clarify that add expects spaces (e.g. @"CLIENT ID") and that with-pipeline handles the conversion automatically.

[carpentry-reviewer]

Build & Tests

CI: All checks pass (macOS + Ubuntu). 77 tests pass.
Local build: Carp compiler not available on review machine. CI verified.

Prior feedback

1. hellerve — rename Pipeline → RedisPipeline: Fully addressed. All references updated consistently across redis.carp and test/resp.carp.
2. hellerve — fix RedisPipeline.add doc string: Addressed. The doc at redis.carp:550–554 now correctly states that add expects spaces (e.g. @"CLIENT ID") and that with-pipeline handles hyphen-to-space conversion. This matches the actual behavior — add uses Pattern.split #" ", while rtreat- (hyphen conversion) only runs in the macro.

Both prior issues are resolved.

Findings

All the pipeline code is correct. I traced through decode-n, pipeline-read, pipeline-exec, and the with-pipeline macro expansion in detail.

1. decode-n is robust (redis.carp:113–127)

Edge cases handled well: n=0 → empty array (for loop never executes), empty string → empty array, truncated data → returns only complete values, position non-advancement guard prevents infinite loops on pathological decode output. Clean integration with decode-one.

2. pipeline-cmd- and with-pipeline macro are correct (redis.carp:597–630)

pipeline-cmd- correctly applies rtreat- and rconv- from defredis, producing properly formatted RedisPipeline.add calls. The __pipeline binding in the let is clean. The cons-last puts pipeline-exec as the final do expression. No bugs.

3. Ownership is clean

RedisPipeline.buf passes by reference to TcpStream.send. count is copied with @. No double-free or use-after-move risks.

4. Minor: pipeline-read potential hang (redis.carp:571)

If the TCP connection delivers perpetually partial RESP data without closing or erroring, decode-n returns [], results never reaches n, and the while loop never exits. This is a pre-existing architectural pattern (the single-command read path has the same risk), so it's not a regression — but worth noting for future hardening.

5. Minor test gaps (test/resp.carp)

• No test for decode-n with n=0 (safe by inspection, but cheap to add).
• No test for RedisPipeline.add with a multi-word command like @"CLIENT ID" to exercise the Pattern.split path. All tests use single-word commands.

Neither is blocking.

Verdict: merge

All prior feedback addressed. Pipeline implementation is correct with clean API design, good code reuse of existing defredis machinery, and thorough test coverage. The minor test gaps and the pre-existing hang risk are not blocking.