Skip to content

Conversation

@sebsto
Copy link
Collaborator

@sebsto sebsto commented Jun 4, 2025

The new streaming code exposes the following elements

public enum ConverseStreamElement: Sendable {
    case messageStart(Role)  // start of a message
    case text(Int, String)  // partial text
    case reasoning(Int, String)  // partial reasoning
    case toolUse(Int, ToolUseBlock)  // a complete tool use response
    case messageComplete(Message)  // complete text message (with all content blocks and reason for stop)
    case metaData(ResponseMetadata)  // metadata about the response
}

@sebsto sebsto requested a review from Copilot June 4, 2025 22:20
@sebsto sebsto self-assigned this Jun 4, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the streaming layer to consolidate and simplify how SDK streaming events are translated into public ConverseStreamElement values. It removes legacy mock stream methods, introduces dedicated test stream generators, and overhauls ConverseReplyStream to use a buffered state machine with logging and new cases (text, reasoning, toolUse, metaData).

  • Remove old mock implementations and add new package-scoped stream generators under Tests/ConverseStream
  • Update ConverseStreamElement enum to new cases and add ResponseMetadata
  • Rewrite ConverseReplyStream to convert raw SDK output into buffered state-driven elements with logging

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
Tests/Mock/MockBedrockRuntimeClient.swift Remove legacy mock stream methods
Tests/ConverseStream/MockBedrockRuntimeClient+StreamGenerator.swift Add new package-scoped stream generators
Tests/ConverseStream/*.swift Delete old vision/text/document tests, update tool/reasoning tests
Sources/Converse/Streaming/ToolUseStart.swift Rename toolUseId to id, adjust initializers
Sources/Converse/Streaming/ResponseMetaData.swift Introduce ResponseMetadata and nested types
Sources/Converse/Streaming/ConverseStreamElement.swift Update enum cases (messageStart, text, reasoning, toolUse, metaData)
Sources/Converse/Streaming/ConverseReplyStream.swift Rewrite stream conversion logic with buffered state, logging, and flush helper
Sources/Converse/Message.swift Add StopReason, record stop reason in Message
Comments suppressed due to low confidence (1)

Tests/ConverseStream/ConverseStreamToolTests.swift:22

  • [nitpick] The file name ConverseStreamToolTests.swift suggests tests for stream tooling, but it extends ConverseReplyStreamTests. Consider renaming the file or matching the extension target to keep names consistent.
extension ConverseReplyStreamTests {

@sebsto sebsto merged commit 76ad933 into main Jun 5, 2025
11 checks passed
@sebsto sebsto deleted the sebsto/new_streaming branch June 5, 2025 07:30
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