Support ADR 50 Fast Ingest with constants

[scottf]

No description provided.

[synadia-claude-reviewer]

Claude finished @scottf's task in 2m 3s —— View job

---

PR Review

• Gather context
• Read changed files
• Review for correctness, security, and performance
• Post findings

---

Summary

This is a pure constants PR — no logic, no thread-safety or exception-handling concerns, and no security issues. The additions are well-organised and the comments are helpful.

Three observations worth verifying against ADR 50 / the server spec:

1. NATS_BATCH_COMMIT_STORE = "1" — The value "1" is not self-documenting. If this is a literal wire value mandated by the server protocol, a short note (e.g., // server wire value for "store" semantics) would prevent future readers from second-guessing it. See inline comment.

2. Error code gaps in atomic batch publish — 10178 is missing between 10177 and 10179, and 10180–10198 are skipped entirely before 10199. If these codes exist server-side but are intentionally unhandled by the client, a comment to that effect would help. See inline comment.

3. Error code gap in fast ingest batch publish — JS_BATCH_PUBLISH_TOO_MANY_INFLIGHT jumps from 10208 to 10211, skipping 10209 (which isn't claimed by either group). Worth confirming no constant was accidentally omitted. See inline comment.

None of these block correctness if the values match the server — they're documentation/completeness concerns.

[synadia-claude-reviewer]

The value "1" for NATS_BATCH_COMMIT_STORE is not immediately obvious. Could this be a boolean-ish wire value (1 = true/store)? A brief comment noting the exact wire format or a link to the ADR/spec would help future readers — the name implies "store" but the value doesn't make that self-evident.

[scottf]

Good instinct - "1" is exactly a boolean true. Expanded the comment block above the constants: the header's presence marks the commit message, the value selects the mode, and "1" is the original 2.12 value from before eob existed. Skipped an ADR link since no other constant in this file carries one.

[synadia-claude-reviewer]

Error code 10178 is missing between JS_ATOMIC_PUBLISH_UNSUPPORTED_HEADER (10177) and JS_ATOMIC_PUBLISH_INVALID_BATCH_ID (10179). Similarly, 10180–10198 are skipped before 10199. If these are intentionally unhandled by the client (server-internal or reserved), a brief comment would prevent future contributors from wondering if they were accidentally omitted.

[scottf]

10178 is JSConsumerPushWithPriorityGroupErr (push consumers + priority groups), not a batch error, so it's correctly absent - same for most of 10180-10197.

But checking that range turned up 10198 JSMirrorWithAtomicPublishErr, which is ours and was missing. Added as JS_MIRROR_WITH_ATOMIC_PUBLISH.

Not annotating the remaining gaps: this is a curated set of batch-publish codes, not a contiguous range, and comments about other features' codes would rot here.

[synadia-claude-reviewer]

Code 10209 is absent in both the atomic and batch-publish error groups. JS_BATCH_PUBLISH_TOO_MANY_INFLIGHT jumps from 10208 to 10211 (10209/10210 are already taken by atomic publish). Intentional? If so, a comment helps. If not, it may indicate a missing constant.

[scottf]

Correct, though not for the stated reason - 10209 is JSMirrorWithBatchPublishErr, not a code taken by atomic publish. Added as JS_MIRROR_WITH_BATCH_PUBLISH, alongside its atomic twin 10198.

Both come from ADR-50's "Mirrors and Sources" section; I'd pulled error codes from the publish path only and missed the stream-config ones. The full set now matches every batch/atomic code in the server's errors.json - 16 of 16, verified programmatically.

[piotrpio]

LGTM!