Skip to content

Conversation

@Rishirandhawa
Copy link

@Rishirandhawa Rishirandhawa commented Nov 14, 2025

Adds a 'static bound to the CompletionClient::CompletionModel associated type so implementations can be turned into trait objects (Box/Arc<dyn ...>). This addresses compiler lifetime/bound errors when converting CompletionModels into dynamic trait objects for CompletionClientDyn/AgentBuilder.
Notes: Most provider implementations already satisfy this because they bound their inner generic HTTP client with 'static

Added 'static lifetime requirement to CompletionModel.Adds a 'static bound to the CompletionClient::CompletionModel associated type so implementations can be turned into trait objects (Box/Arc<dyn ...>)
Copy link
Collaborator

@joshua-mo-143 joshua-mo-143 left a comment

Choose a reason for hiding this comment

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

Only one change required then lgtm

pub trait CompletionClient: ProviderClient + Clone {
/// The type of CompletionModel used by the client.
type CompletionModel: CompletionModel;
// require the associated CompletionModel to be 'static so it can be used
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use the original docstring here and move this detail to the trait docstrings rather than having it here

@Sytten
Copy link
Contributor

Sytten commented Nov 21, 2025

I mean we already have CompletionClientDyn for this, unless you want to get rid of the dyn version @joshua-mo-143 I would probably not impose that constraint on the trait.

@joshua-mo-143
Copy link
Collaborator

I mean we already have CompletionClientDyn for this, unless you want to get rid of the dyn version @joshua-mo-143 I would probably not impose that constraint on the trait.

Ah, yeah - completely forgot about that.

@Rishirandhawa Is there any specific reason why you need this specific change rather than CompletionClientDyn?

@joshua-mo-143
Copy link
Collaborator

Closing in the interest of repo hygiene since there's no response and there was no prior discussion.

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.

3 participants