Skip to content

Conversation

@dicksontsai
Copy link
Collaborator

  1. Refactored query() out of src/claude_code_sdk/__init__.py
  2. Refactored parse_message() out of src/claude_code_sdk/_internal/client.py
  3. Added src/claude_code_sdk/client.py
  4. Added examples and tests

@dicksontsai dicksontsai requested a review from ltawfik July 19, 2025 21:00
@wolffiex

This comment was marked as resolved.

@claude

This comment was marked as resolved.

Copy link
Collaborator

@wolffiex wolffiex left a comment

Choose a reason for hiding this comment

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

Code Review

Great implementation of streaming capabilities! The bidirectional streaming API is well-designed with clear separation between query() for simple use cases and ClaudeSDKClient for interactive sessions.

Key strengths:

  • Clean API design with two distinct modes
  • Good async/await patterns throughout
  • Comprehensive test coverage
  • Helpful examples

Areas for improvement:

1. JSON Error Handling

  • JSON parsing errors are silently ignored in subprocess_cli.py:294
  • No error handling for missing keys in message_parser.py
  • Inconsistent use of .get() vs direct key access

2. Parameter Naming Inconsistencies

  • connect() uses 'prompt' while send_message() uses 'content'
  • 'prompt' is confusing for AsyncIterable of messages
  • Should be consistent across the API

3. Resource Management

  • Hardcoded timeouts (30s for control requests)
  • Fixed memory limits not configurable
  • No backpressure handling

4. API Clarity

  • receive_response() stopping behavior could be clearer
  • send_message() only accepts strings vs structured messages in connect()

Overall, this is a solid foundation that would benefit from addressing these consistency and robustness issues.

@dicksontsai
Copy link
Collaborator Author

Addressing feedback:

JSON parsing errors are silently ignored in subprocess_cli.py:294

The existing code in subprocess_cli.py:294 tries to decode a buffer until it encounters a valid JSON object, so silently ignoring JSON decoding errors is intentional. This PR maintains this behavior for backward compatibility.

No error handling for missing keys in message_parser.py
Inconsistent use of .get() vs direct key access

Added KeyError checking in message_parser.py to match the Typescript SDK. All direct key accesses are now wrapped.

connect() uses 'prompt' while send_message() uses 'content'
'prompt' is confusing for AsyncIterable of messages
Should be consistent across the API

Renamed ClaudeSDKClient.send_message() to ClaudeSDKClient.query(), and named the parameter prompt across ClaudeSDKClient.connect(), ClaudeSDKClient.query(), and query(). I stick with prompt because that's what the Typescript SDK uses.

  1. Resource Management
    Hardcoded timeouts (30s for control requests)

Fixed

Fixed memory limits not configurable
No backpressure handling

Let's address this in a separate PR. This PR should focus on providing a new way to feed input into Claude Code SDK via streaming. I believe the "fixed memory limits" and "backpressure" issues refer to the output stream, which this PR should ideally not touch.

  1. API Clarity
    receive_response() stopping behavior could be clearer

Fixed

send_message() only accepts strings vs structured messages in connect()

Fixed

@dicksontsai dicksontsai requested a review from wolffiex July 20, 2025 03:43
@dicksontsai dicksontsai merged commit c4384ea into main Jul 20, 2025
6 checks passed
@dicksontsai dicksontsai deleted the dickson/streaming branch July 20, 2025 15:10
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.

4 participants