Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/agentlab/llm/chat_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -553,3 +553,43 @@ def make_model(self):
temperature=self.temperature,
max_tokens=self.max_new_tokens,
)


class BedrockChatModel(AnthropicChatModel):
def __init__(
self,
model_name,
api_key=None,
temperature=0.5,
max_tokens=100,
max_retry=4,
):
Comment on lines +558 to +566
Copy link

Choose a reason for hiding this comment

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

Misleading Parameter Inheritance category Readability

Tell me more
What is the issue?

The BedrockChatModel accepts an unused api_key parameter inherited from AnthropicChatModel.

Why this matters

Including unused parameters creates confusion about the actual requirements of the class and its relationship with the parent class.

Suggested change ∙ Feature Preview
class BedrockChatModel(AnthropicChatModel):
    def __init__(
        self,
        model_name,
        temperature=0.5,
        max_tokens=100,
        max_retry=4,
    ):
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

self.model_name = model_name
self.temperature = temperature
self.max_tokens = max_tokens
self.max_retry = max_retry
Comment on lines +558 to +570
Copy link

Choose a reason for hiding this comment

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

Improper inheritance implementation category Design

Tell me more
What is the issue?

BedrockChatModel inherits from AnthropicChatModel but completely overrides the parent's init method without calling super().init(). This breaks the inheritance chain and duplicates code.

Why this matters

This violates the DRY principle and could lead to maintenance issues if the parent class initialization changes. It also makes the inheritance relationship misleading since the child isn't actually using the parent's initialization logic.

Suggested change ∙ Feature Preview
class BedrockChatModel(AnthropicChatModel):
    def __init__(self, *args, **kwargs):
        if not all(os.getenv(key) for key in ['AWS_REGION', 'AWS_ACCESS_KEY', 'AWS_SECRET_KEY']):
            raise ValueError('AWS credentials must be set in environment')
            
        super().__init__(*args, **kwargs)
        self.client = anthropic.AnthropicBedrock(
            aws_region=os.getenv('AWS_REGION'),
            aws_access_key=os.getenv('AWS_ACCESS_KEY'),
            aws_secret_key=os.getenv('AWS_SECRET_KEY')
        )
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.


if (
not os.getenv("AWS_REGION")
or not os.getenv("AWS_ACCESS_KEY")
or not os.getenv("AWS_SECRET_KEY")
Comment on lines +574 to +575
Copy link

Choose a reason for hiding this comment

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

Incorrect AWS environment variable names category Functionality

Tell me more
What is the issue?

The AWS environment variable names used are non-standard. AWS SDK typically expects AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY, not AWS_ACCESS_KEY and AWS_SECRET_KEY.

Why this matters

Using incorrect environment variable names will cause authentication failures even when users have properly configured their AWS credentials using standard AWS conventions.

Suggested change ∙ Feature Preview

Use the standard AWS environment variable names:

if (
    not os.getenv("AWS_REGION")
    or not os.getenv("AWS_ACCESS_KEY_ID")
    or not os.getenv("AWS_SECRET_ACCESS_KEY")
):
    raise ValueError(
        "AWS_REGION, AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY must be set in the environment when using BedrockChatModel"
    )

self.client = anthropic.AnthropicBedrock(
    aws_region=os.getenv("AWS_REGION"),
    aws_access_key=os.getenv("AWS_ACCESS_KEY_ID"),
    aws_secret_key=os.getenv("AWS_SECRET_ACCESS_KEY"),
)
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

):
raise ValueError(
"AWS_REGION, AWS_ACCESS_KEY and AWS_SECRET_KEY must be set in the environment when using BedrockChatModel"
)

self.client = anthropic.AnthropicBedrock(
aws_region=os.getenv("AWS_REGION"),
aws_access_key=os.getenv("AWS_ACCESS_KEY"),
aws_secret_key=os.getenv("AWS_SECRET_KEY"),
)
Comment on lines +572 to +585
Copy link

Choose a reason for hiding this comment

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

Insecure storage of AWS credentials category Security

Tell me more
What is the issue?

The code is using environment variables to store and retrieve AWS credentials, which is not the most secure method for handling sensitive information.

Why this matters

Storing credentials in environment variables can lead to accidental exposure through system logs or environment dumps. If the system is compromised, an attacker could potentially access these credentials.

Suggested change ∙ Feature Preview

Use AWS IAM roles for EC2 instances or AWS SDK credential providers instead of environment variables. If environment variables must be used, consider using a secure secrets management system like AWS Secrets Manager or HashiCorp Vault.

Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.



@dataclass
class BedrockModelArgs(BaseModelArgs):
def make_model(self):
return BedrockChatModel(
model_name=self.model_name,
temperature=self.temperature,
max_tokens=self.max_new_tokens,
)
17 changes: 17 additions & 0 deletions src/agentlab/llm/llm_configs.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from agentlab.llm.chat_api import (
AnthropicModelArgs,
AzureModelArgs,
BedrockModelArgs,
OpenAIModelArgs,
OpenRouterModelArgs,
SelfHostedModelArgs,
Expand Down Expand Up @@ -209,6 +210,22 @@
max_new_tokens=16_384,
temperature=1e-1,
),
# ------------ Anthropic / Bedrock ------------#
"bedrock/claude-3-7-sonnet": BedrockModelArgs(
model_name="us.anthropic.claude-3-7-sonnet-20250219-v1:0",
max_new_tokens=16_384,
temperature=1e-1,
),
"bedrock/claude-4-0-sonnet": BedrockModelArgs(
model_name="us.anthropic.claude-sonnet-4-20250514-v1:0",
max_new_tokens=16_384,
temperature=1e-1,
),
"bedrock/claude-4-5-sonnet": BedrockModelArgs(
model_name="us.anthropic.claude-sonnet-4-5-20250929-v1:0",
max_new_tokens=16_384,
temperature=1e-1,
),
# ---------------- OSS LLMs ----------------#
"meta-llama/Meta-Llama-3-70B-Instruct": SelfHostedModelArgs(
model_name="meta-llama/Meta-Llama-3-70B-Instruct",
Expand Down
Loading