Skip to content

Conversation

@lalinsky
Copy link
Owner

@lalinsky lalinsky commented Aug 24, 2025

Summary

  • Implement missing $JS.API.STREAM.INFO.{stream} endpoint in JetStream API
  • Add getStreamInfo method to jetstream.zig with comprehensive test coverage
  • Reorganize JetStream tests by moving stream management tests to dedicated file

Changes Made

JetStream API Implementation

  • ✅ Added getStreamInfo(stream_name) method to JetStream struct in src/jetstream.zig:367-376
  • ✅ Method follows existing patterns: builds API subject, sends request, parses response
  • ✅ Returns Result(StreamInfo) for consistency with other JetStream methods

Test Organization

  • ✅ Created dedicated tests/jetstream_stream_test.zig for stream management tests
  • ✅ Moved 6 stream-related tests from jetstream_test.zig to new file:
    • test "list stream names" - Tests $JS.API.STREAM.NAMES
    • test "add stream" - Tests $JS.API.STREAM.CREATE.{stream}
    • test "list streams" - Tests $JS.API.STREAM.LIST
    • test "update stream" - Tests $JS.API.STREAM.UPDATE.{stream}
    • test "delete stream" - Tests $JS.API.STREAM.DELETE.{stream}
    • test "get stream info" - Tests $JS.API.STREAM.INFO.{stream}NEW
  • ✅ Updated tests/all_tests.zig to include new test module
  • ✅ Kept consumer management tests in jetstream_test.zig

Test Results

All tests pass: 27/27

  • New jetstream_stream_test.test.get stream info runs successfully (10.34ms)
  • No existing functionality broken
  • Clean test organization by functional area

Technical Notes

This addresses one of the missing JetStream API endpoints identified in the codebase analysis. The STREAM.INFO endpoint is commonly used for monitoring and management operations, providing stream configuration and state information without requiring create/update operations.

Summary by CodeRabbit

  • New Features

    • Added a JetStream capability to retrieve detailed information for a specific stream, enabling clients to inspect stream configuration and status.
  • Tests

    • Introduced a comprehensive stream lifecycle test suite covering add, list names, list, update, delete, and get info.
    • Reorganized tests by moving stream management checks into a dedicated suite and removing them from the previous general JetStream tests.

- Implement getStreamInfo method in jetstream.zig for $JS.API.STREAM.INFO endpoint
- Add comprehensive test for STREAM.INFO functionality
- Reorganize JetStream tests by moving stream management tests to dedicated jetstream_stream_test.zig file
- Update test runner to include new test file
- All tests passing (27/27)

This addresses the missing STREAM.INFO endpoint identified in the JetStream API implementation.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 24, 2025

Walkthrough

Introduces JetStream.getStreamInfo to request and parse stream info via STREAM.INFO.{name}. Adds a new test suite for stream lifecycle and info retrieval, updates the central test aggregator to include it, and removes overlapping stream tests from the existing JetStream test file.

Changes

Cohort / File(s) Summary
JetStream API
src/jetstream.zig
Added pub fn getStreamInfo(self: *JetStream, stream_name: []const u8) !Result(StreamInfo) that builds STREAM.INFO.{s}, sends an empty payload request, parses response, and frees allocated subject.
Test harness aggregation
tests/all_tests.zig
Added pub const jetstream_stream_tests = @import("jetstream_stream_test.zig"); to include new stream tests.
Stream lifecycle tests (new)
tests/jetstream_stream_test.zig
New tests covering add/list/update/delete stream operations and getStreamInfo, including setup/teardown and config assertions.
JetStream tests cleanup
tests/jetstream_test.zig
Removed stream management tests; retained other JetStream tests (connect, account info, consumer ops).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Caller
  participant JS as JetStream
  participant NC as NATS Conn
  participant SV as NATS Server

  Dev->>JS: getStreamInfo(stream_name)
  JS->>JS: Build subject "STREAM.INFO.{name}"
  JS->>NC: Request(subject, payload="")
  NC->>SV: PUB/INBOX (request)
  SV-->>NC: MSG (stream info JSON)
  NC-->>JS: Response bytes
  JS->>JS: parseResponse -> StreamInfo
  JS-->>Dev: Result(StreamInfo)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

A whisker twitch, a subject spun,
STREAM.INFO hops—request is done.
Tests now burrow, neat and tight,
Streams appear, then take their flight.
With ears up high, I parse and cheer—
JetStream’s facts are crystal clear. 🐇✨

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/jetstream-stream-info

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (6)
src/jetstream.zig (1)

366-376: STREAM.INFO implementation looks solid and consistent with existing JetStream methods

  • Correct subject construction, allocation, and cleanup.
  • Uses the same parseResponse/Result pattern as add/update/delete stream.

Optional: de-duplicate stream-subject building across methods

You build "STREAM.{ACTION}.{name}" in several places (CREATE/UPDATE/DELETE/INFO). Consider a small helper to reduce repetition and centralize the format.

Apply this diff within the changed block:

-        // Build the subject for the API call
-        const subject = try std.fmt.allocPrint(self.allocator, "STREAM.INFO.{s}", .{stream_name});
-        defer self.allocator.free(subject);
-
-        const msg = try self.sendRequest(subject, "");
+        const msg = try self.sendStreamRequest("INFO", stream_name, "");
         defer msg.deinit();

Add this helper elsewhere in the file (outside the selected range):

fn sendStreamRequest(self: *JetStream, action: []const u8, stream_name: []const u8, payload: []const u8) !*Message {
    const subject = try std.fmt.allocPrint(self.allocator, "STREAM.{s}.{s}", .{ action, stream_name });
    defer self.allocator.free(subject);
    return try self.sendRequest(subject, payload);
}
tests/jetstream_stream_test.zig (5)

8-40: List stream names test is clear and correct

  • Creates a stream, lists names, and asserts presence. Matches API semantics and cleans up resources.

Optional: assert uniqueness to catch cross-test leakage

Since beforeEach purges all streams, you could also assert that result.value.len == 1 here to detect unexpected leftover streams. Up to you given the global cleanup.


42-71: Add stream test verifies key config fields

  • Checks name, subjects, retention, storage, and limits — good coverage for creation response shape.

Optional: add a couple more invariants

  • Assert num_replicas and max_bytes to fully mirror the requested config.

72-111: List streams test validates returned configs

  • Finds the created stream and inspects important fields.

Optional: verify that total equals returned count in paged response

Your library currently collapses the page to a flat slice and asserts total == len internally; adding a test assertion here would guard against regressions if pagination handling changes.


112-147: Update stream test covers the happy path well

  • Create, assert initial, update, assert new value — straightforward and reliable.

Optional: assert that unrelated fields remain unchanged after update

E.g., subjects or retention, to ensure partial updates don't accidentally reset fields.


148-193: Delete stream test validates pre/post conditions via listing

  • Good approach to verify deletion without relying on error types.

Optional: also assert getStreamInfo fails post-delete when typed errors become available

If/when the client exposes a more granular JetStream error set, add:

  • try testing.expectError(nats.JetStreamError.StreamNotFound, js.getStreamInfo("TEST_DELETE_STREAM"));

I can help wire specific JetStream error codes through maybeParseErrorResponse so tests can assert finer-grained errors. Want a follow-up PR?

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6891053 and fb59e59.

📒 Files selected for processing (4)
  • src/jetstream.zig (1 hunks)
  • tests/all_tests.zig (1 hunks)
  • tests/jetstream_stream_test.zig (1 hunks)
  • tests/jetstream_test.zig (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-23T20:26:30.660Z
Learnt from: CR
PR: lalinsky/nats.zig#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-23T20:26:30.660Z
Learning: Run tests (zig build test) to confirm your changes are working

Applied to files:

  • tests/jetstream_stream_test.zig
🔇 Additional comments (3)
tests/all_tests.zig (1)

11-11: Good test split: importing jetstream_stream_test.zig keeps suites focused

The new import cleanly separates stream lifecycle tests from consumer tests, improving organization without altering the harness.

tests/jetstream_test.zig (1)

238-238: No functional change

The added blank line is fine and does not affect test behavior.

tests/jetstream_stream_test.zig (1)

194-223: Unable to locate the test file path automatically—please verify that tests/jetstream_stream_test.zig exists and is named correctly. Without access to Zig or a fully cloned tree here, I cannot confirm those optional state fields exist.

Next steps for manual verification

  • Ensure that your getStreamInfo return type includes
    state.messages
    state.consumer_count
  • If so, consider adding these zero-value assertions:
    try testing.expect(retrieved_info.value.state.messages == 0);
    try testing.expect(retrieved_info.value.state.consumer_count == 0);
  • Finally, run the full test suite locally and confirm all tests pass:
    zig build test --summary all

@lalinsky lalinsky merged commit a2fd523 into main Aug 24, 2025
3 checks passed
@lalinsky lalinsky deleted the feature/jetstream-stream-info branch August 24, 2025 15:17
@coderabbitai coderabbitai bot mentioned this pull request Sep 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant