Skip to content

Conversation

@HenryL27
Copy link
Collaborator

@HenryL27 HenryL27 commented Apr 9, 2025

No description provided.

@HenryL27 HenryL27 requested a review from eric-anderson April 9, 2025 22:05
kwargs = {"model_name": self.model_name, "cache": self._cache, "default_mode": self._default_mode}
return deserializer, (kwargs,)

def default_mode(self) -> LLMMode:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can simplify this. Have each subclass pass in default_mode as a parameter to the LLM superclass (already done), and then have the LLM class return self._default_mode always.

"""Abstract representation of an LLM instance. and should be subclassed to implement specific LLM providers."""

def __init__(self, model_name, cache: Optional[Cache] = None):
def __init__(self, model_name, cache: Optional[Cache] = None, default_mode: Optional[LLMMode] = None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

make default_mode not optional here. You could do = LLMMode.SYNC, but given how few subclasses we have I think it's better to have it be explicit.

Signed-off-by: Henry Lindeman <[email protected]>
@HenryL27 HenryL27 enabled auto-merge (squash) April 10, 2025 01:25
@HenryL27 HenryL27 merged commit eb79f53 into main Apr 10, 2025
11 of 15 checks passed
@HenryL27 HenryL27 deleted the hml-default-llm-mode branch April 10, 2025 04:22
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