Skip to content

Conversation

@sebsto
Copy link
Collaborator

@sebsto sebsto commented Jun 7, 2025

@sebsto sebsto requested a review from Copilot June 7, 2025 18:55
@sebsto sebsto self-assigned this Jun 7, 2025
@sebsto sebsto mentioned this pull request Jun 7, 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

Adds the ability to prefix model IDs at runtime for cross-region Bedrock calls and updates existing models and invocation flows to use this new mechanism.

  • Introduce CrossRegionInferenceModality protocol and apply it to supported model structs
  • Strip hard-coded region prefixes from static model IDs and rely on getModelIdWithCrossRegionInferencePrefix(region:)
  • Update request builders and service methods to accept a Region and prepend the region prefix dynamically

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Sources/Modalities/CrossRegionInference.swift New protocol defining how to generate region prefixes
Sources/Models/* Add CrossRegionInferenceModality conformance and remove static IDs
Sources/BedrockModel.swift Implement getModelIdWithCrossRegionInferencePrefix(region:)
Sources/InvokeModel/.swift & Sources/Converse/.swift Change get*Input() signatures to accept a Region
Sources/ListModels/ModelSummary.swift Remove default "us." fallback when mapping SDK summaries
README.md & Examples/* Update static examples to remove hard-coded region prefixes
Comments suppressed due to low confidence (3)

Sources/InvokeModel/InvokeModelRequest.swift:208

  • Changing the public API signature for getInvokeModelInput is a breaking change. To maintain backward compatibility, provide an overload or deprecate the old method before removal.
public func getInvokeModelInput(forRegion region: Region) throws -> InvokeModelInput {

Sources/Modalities/CrossRegionInference.swift:16

  • [nitpick] Consider adding documentation comments to CrossRegionInferenceModality and its crossRegionPrefix(forRegion:) method to explain how prefixes are determined and when to adopt this protocol.
public protocol CrossRegionInferenceModality: Sendable {}

Sources/ListModels/ModelSummary.swift:62

  • Removing the fallback prefix (us.\(modelId)) may cause bedrockModel to be nil for existing region-specific models. Consider reintroducing a default region prefix or using the current region context so that mapping still succeeds when listing models.
let bedrockModel = BedrockModel(rawValue: modelId)

@sebsto sebsto merged commit 22d011b into main Jun 7, 2025
16 checks passed
@sebsto sebsto deleted the sebsto/cross-region-inference branch June 7, 2025 19:05
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