-
Notifications
You must be signed in to change notification settings - Fork 2
add a text chat example #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this 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 adds a new interactive text chat example using the Swift BedrockService, complete with build configuration and CI integration.
- Implements a streaming CLI example that maintains conversation history.
- Introduces a standalone Swift package and
.gitignorefor the example. - Updates the GitHub Actions workflow to include
text_chatin the examples matrix.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Examples/text_chat/Sources/TextChat.swift | New CLI example for streaming text chat with conversation history |
| Examples/text_chat/Package.swift | Package manifest for building the TextChat executable |
| Examples/text_chat/.gitignore | Workspace and build artifact ignores for the example |
| .github/workflows/pull_request.yml | Adds text_chat to the CI job’s examples list |
Comments suppressed due to low confidence (1)
Examples/text_chat/Sources/TextChat.swift:98
- [nitpick] Consider renaming 'MyError' to something more descriptive like 'TextChatError' to clarify its purpose.
enum MyError: Error {
| .withPrompt(prompt) | ||
| } else { | ||
| // append the new prompt to the existing request | ||
| // ConverseRequestBuilder is stateles, it doesn't keep track of the history |
Copilot
AI
Jun 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in comment: 'stateles' should be 'stateless'.
| // ConverseRequestBuilder is stateles, it doesn't keep track of the history | |
| // ConverseRequestBuilder is stateless, it doesn't keep track of the history |
| } else { | ||
| // append the new prompt to the existing request | ||
| // ConverseRequestBuilder is stateles, it doesn't keep track of the history | ||
| request = try ConverseRequestBuilder(from: request!) |
Copilot
AI
Jun 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Force-unwrapping 'request' may cause a runtime crash if it's nil; consider using a guard let to safely unwrap or refactor the logic to avoid optionals here.
| request = try ConverseRequestBuilder(from: request!) | |
| guard let existingRequest = request else { | |
| throw MyError.incorrectModality("Request is unexpectedly nil") | |
| } | |
| request = try ConverseRequestBuilder(from: existingRequest) |
| history.append(Message(prompt)) | ||
|
|
||
| // send the request | ||
| let reply = try await bedrock.converseStream(with: request!) |
Copilot
AI
Jun 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Force-unwrapping 'request' here may crash if nil; ensure it's non-nil with a guard or unwrap earlier.
| let reply = try await bedrock.converseStream(with: request!) | |
| guard let unwrappedRequest = request else { | |
| throw MyError.incorrectModality("Request is unexpectedly nil") | |
| } | |
| let reply = try await bedrock.converseStream(with: unwrappedRequest) |
| do { | ||
| try await TextChat.run() | ||
| } catch { | ||
| print("Error:\n\(error)") |
Copilot
AI
Jun 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider using 'logger.error' instead of 'print' for error messages to maintain consistent logging and support structured logs.
| do { | |
| try await TextChat.run() | |
| } catch { | |
| print("Error:\n\(error)") | |
| var logger = Logger(label: "TextChat.Main") | |
| do { | |
| try await TextChat.run() | |
| } catch { | |
| logger.error("Error:\n\(error)") |
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.