ronantakizawa/codereview-bench huggingface dataset:
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/autogpt_libs/autogpt_libs/supabase_integration_credentials_store/types.py (Python)

Code before:
from typing import Annotated, Any, Literal, Optional, TypedDict
from uuid import uuid4

from pydantic import BaseModel, Field, SecretStr, field_serializer


class _BaseCredentials(BaseModel):
    id: str = Field(default_factory=lambda: str(uuid4()))
    provider: str
    title: Optional[str]
    hidden: Optional[bool] = False

    @field_serializer("*")
    def dump_secret_strings(value: Any, _info):
        if isinstance(value, SecretStr):
            return value.get_secret_value()
        return value


class OAuth2Credentials(_BaseCredentials):
    type: Literal["oauth2"] = "oauth2"
    username: Optional[str]
    """Username of the third-party service user that these credentials belong to"""
    access_token: SecretStr
    access_token_expires_at: Optional[int]
    """Unix timestamp (seconds) indicating when the access token expires (if at all)"""
    refresh_token: Optional[SecretStr]
    refresh_token_expires_at: Optional[int]
    """Unix timestamp (seconds) indicating when the refresh token expires (if at all)"""
    scopes: list[str]
    metadata: dict[str, Any] = Field(default_factory=dict)

    def bearer(self) -> str:
        return f"Bearer {self.access_token.get_secret_value()}"



Review comment (security):
1. "Hidden" isn't the right name for this, since it doesn't generally hide the item except on a specific screen.
2. Putting this here will also add and persist it to the DB on all new credentials created. This isn't necessary and should be avoided if possible.

Code after:
from typing import Annotated, Any, Literal, Optional, TypedDict
from uuid import uuid4

from pydantic import BaseModel, Field, SecretStr, field_serializer


class _BaseCredentials(BaseModel):
    id: str = Field(default_factory=lambda: str(uuid4()))
    provider: str
    title: Optional[str]

    @field_serializer("*")
    def dump_secret_strings(value: Any, _info):
        if isinstance(value, SecretStr):
            return value.get_secret_value()
        return value


class OAuth2Credentials(_BaseCredentials):
    type: Literal["oauth2"] = "oauth2"
    username: Optional[str]
    """Username of the third-party service user that these credentials belong to"""
    access_token: SecretStr
    access_token_expires_at: Optional[int]
    """Unix timestamp (seconds) indicating when the access token expires (if at all)"""
    refresh_token: Optional[SecretStr]
    refresh_token_expires_at: Optional[int]
    """Unix timestamp (seconds) indicating when the refresh token expires (if at all)"""
    scopes: list[str]
    metadata: dict[str, Any] = Field(default_factory=dict)

    def bearer(self) -> str:
        return f"Bearer {self.access_token.get_secret_value()}"


class APIKeyCredentials(_BaseCredentials):
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/backend/backend/data/model.py (Python)

Code before:


CredentialsType = Literal["api_key", "oauth2"]


class OAuthState(BaseModel):
    token: str
    provider: str
    expires_at: int
    scopes: list[str]
    """Unix timestamp (seconds) indicating when this OAuth state expires"""


class UserMetadata(BaseModel):
    integration_credentials: list[Credentials] = Field(default_factory=list)
    integration_oauth_states: list[OAuthState] = Field(default_factory=list)


class UserMetadataRaw(TypedDict, total=False):
    integration_credentials: list[dict]
    integration_oauth_states: list[dict]


class UserIntegrations(BaseModel):
    credentials: list[Credentials] = Field(default_factory=list)
    oauth_states: list[OAuthState] = Field(default_factory=list)


CP = TypeVar("CP", bound=str)
CT = TypeVar("CT", bound=CredentialsType)


CREDENTIALS_FIELD_NAME = "credentials"


class CredentialsMetaInput(BaseModel, Generic[CP, CT]):
    id: str
    title: Optional[str] = None
    provider: CP
    type: CT


class CredentialsFieldSchemaExtra(BaseModel, Generic[CP, CT]):
    # TODO: move discrimination mechanism out of CredentialsField (frontend + backend)
    credentials_provider: list[CP]
    credentials_scopes: Optional[list[str]]
    credentials_types: list[CT]
    discriminator: Optional[str] = None
    discriminator_mapping: Optional[dict[str, CP]] = None



Review comment (suggestion):
Not a huge fan of how `backend.data.model` is becoming a grab bag of "other" stuff.

All of this is stored on the `User` model, so maybe it should be in `backend.data.user` instead? Or even better: make a `backend.data.user.integrations` module.

Code after:
    Field(discriminator="type"),
]


CredentialsType = Literal["api_key", "oauth2"]


class OAuthState(BaseModel):
    token: str
    provider: str
    expires_at: int
    """Unix timestamp (seconds) indicating when this OAuth state expires"""
    scopes: list[str]


class UserMetadata(BaseModel):
    integration_credentials: list[Credentials] = Field(default_factory=list)
    integration_oauth_states: list[OAuthState] = Field(default_factory=list)


class UserMetadataRaw(TypedDict, total=False):
    integration_credentials: list[dict]
    integration_oauth_states: list[dict]


class UserIntegrations(BaseModel):
    credentials: list[Credentials] = Field(default_factory=list)
    oauth_states: list[OAuthState] = Field(default_factory=list)


CP = TypeVar("CP", bound=str)
CT = TypeVar("CT", bound=CredentialsType)


CREDENTIALS_FIELD_NAME = "credentials"


class CredentialsMetaInput(BaseModel, Generic[CP, CT]):
    id: str
    title: Optional[str] = None
    provider: CP
    type: CT


class CredentialsFieldSchemaExtra(BaseModel, Generic[CP, CT]):
    # TODO: move discrimination mechanism out of CredentialsField (frontend + backend)
    credentials_provider: list[CP]
    credentials_scopes: Optional[list[str]]
    credentials_types: list[CT]
    discriminator: Optional[str] = None
    discriminator_mapping: Optional[dict[str, CP]] = None
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/backend/backend/util/json.py (Python)

Code before:
    return jsonable_encoder(data)


def dumps(data) -> str:
    return json.dumps(jsonable_encoder(data))


T = TypeVar("T")


@overload
def loads(data: str, *args, target_type: Type[T], **kwargs) -> T: ...


@overload
def loads(data: str, *args, **kwargs) -> Any: ...


def loads(data: str, *args, target_type: Type[T] | None = None, **kwargs) -> Any:
    parsed = json.loads(data, *args, **kwargs)
    if target_type:
        return type_match(parsed, target_type)
    return parsed


def validate_jsonschema(schema: dict[str, Any], data: dict[str, Any]) -> str | None:
    """
    Validate the data against the schema.
    Returns the validation error message if the data does not match the schema.
    """
    try:
        jsonschema.validate(data, schema)
        return None
    except jsonschema.ValidationError as e:
        return str(e)


Review comment (suggestion):
```suggestion
def validate(data: dict[str, Any], schema: dict[str, Any]) -> str | None:
```
or
```suggestion
def validate_with_schema(schema: dict[str, Any], data: dict[str, Any]) -> str | None:
```

since it doesn't validate the schema, it validates the data.

Code after:
    return jsonable_encoder(data)


def dumps(data) -> str:
    return json.dumps(jsonable_encoder(data))


T = TypeVar("T")


@overload
def loads(data: str, *args, target_type: Type[T], **kwargs) -> T: ...


@overload
def loads(data: str, *args, **kwargs) -> Any: ...


def loads(data: str, *args, target_type: Type[T] | None = None, **kwargs) -> Any:
    parsed = json.loads(data, *args, **kwargs)
    if target_type:
        return type_match(parsed, target_type)
    return parsed


def validate_with_jsonschema(
    schema: dict[str, Any], data: dict[str, Any]
) -> str | None:
    """
    Validate the data against the schema.
    Returns the validation error message if the data does not match the schema.
    """
    try:
        jsonschema.validate(data, schema)
        return None
    except jsonschema.ValidationError as e:
        return str(e)
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/frontend/src/lib/autogpt-server-api/types.ts (TypeScript)

Code before:

/* Mirror of backend/data/graph.py:GraphExecution */
export type GraphExecution = {
  execution_id: string;
  started_at: number;
  ended_at: number;
  duration: number;
  total_run_time: number;
  status: "INCOMPLETE" | "QUEUED" | "RUNNING" | "COMPLETED" | "FAILED";
  graph_id: string;
  graph_version: number;
};

/* backend/data/graph.py:Graph = GraphMeta & Graph */
export type GraphMeta = {
  id: string;
  version: number;
  is_active: boolean;
  is_template: boolean;
  name: string;
  description: string;
  input_schema: BlockIOObjectSubSchema;
  output_schema: BlockIOObjectSubSchema;
};

export type Graph = GraphMeta & {
  nodes: Array<Node>;
  links: Array<Link>;
};

export type GraphUpdateable = Omit<
  Graph,
  | "version"
  | "is_active"
  | "is_template"
  | "links"
  | "input_schema"
  | "output_schema"
> & {
  version?: number;
  is_active?: boolean;
  is_template?: boolean;
  links: Array<LinkCreatable>;
  input_schema?: BlockIOObjectSubSchema;
  output_schema?: BlockIOObjectSubSchema;
};

export type GraphCreatable = Omit<GraphUpdateable, "id"> & { id?: string };

/* Derived from backend/executor/manager.py:ExecutionManager.add_execution */
export type GraphExecuteResponse = {

Review comment (suggestion):
Now this is a mirror
```suggestion
/* Mirror of backend/data/graph.py:Graph */
export type Graph = GraphMeta & {
```

Code after:

/* Mirror of backend/data/graph.py:GraphExecution */
export type GraphExecution = {
  execution_id: string;
  started_at: number;
  ended_at: number;
  duration: number;
  total_run_time: number;
  status: "INCOMPLETE" | "QUEUED" | "RUNNING" | "COMPLETED" | "FAILED";
  graph_id: string;
  graph_version: number;
};

export type GraphMeta = {
  id: string;
  version: number;
  is_active: boolean;
  is_template: boolean;
  name: string;
  description: string;
  input_schema: BlockIOObjectSubSchema;
  output_schema: BlockIOObjectSubSchema;
};

/* Mirror of backend/data/graph.py:Graph */
export type Graph = GraphMeta & {
  nodes: Array<Node>;
  links: Array<Link>;
};

export type GraphUpdateable = Omit<
  Graph,
  | "version"
  | "is_active"
  | "is_template"
  | "links"
  | "input_schema"
  | "output_schema"
> & {
  version?: number;
  is_active?: boolean;
  is_template?: boolean;
  links: Array<LinkCreatable>;
  input_schema?: BlockIOObjectSubSchema;
  output_schema?: BlockIOObjectSubSchema;
};

export type GraphCreatable = Omit<GraphUpdateable, "id"> & { id?: string };

/* Derived from backend/executor/manager.py:ExecutionManager.add_execution */
export type GraphExecuteResponse = {
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/backend/backend/data/credit.py (Python)

Code before:
    block_id: str | None = None
    block: str | None = None
    input: BlockInput | None = None


class UserTransaction(BaseModel):
    transaction_time: datetime = datetime.min
    transaction_type: CreditTransactionType = CreditTransactionType.USAGE
    amount: int = 0
    balance: int = 0
    description: str | None = None
    usage_graph_id: str | None = None
    usage_execution_id: str | None = None
    usage_node_count: int = 0
    usage_starting_time: datetime = datetime.max


class TransactionHistory(BaseModel):
    transactions: list[UserTransaction]
    next_transaction_time: datetime | None


class AutoTopUpConfig(BaseModel):
    amount: int
    """Amount of credits to top up."""
    threshold: int
    """Threshold to trigger auto top up."""


class UserCreditBase(ABC):
    @abstractmethod
    async def get_credits(self, user_id: str) -> int:
        """
        Get the current credits for the user.

        Returns:
            int: The current credits for the user.
        """
        pass

    @abstractmethod
    async def get_transaction_history(
        self,
        user_id: str,
        transaction_time: datetime,
        transaction_count_limit: int,
    ) -> TransactionHistory:
        """
        Get the credit transactions for the user.

        Args:

Review comment (suggestion):
Something to think about; users may want to filter by transaction type or all transactions for specific period (for accounting).

Code after:
        Get the current credits for the user.

        Returns:
            int: The current credits for the user.
        """
        pass

    @abstractmethod
    async def get_transaction_history(
        self,
        user_id: str,
        transaction_time: datetime,
        transaction_count_limit: int,
    ) -> TransactionHistory:
        """
        Get the credit transactions for the user.

        Args:
            user_id (str): The user ID.
            transaction_time (datetime): The upper bound of the transaction time.
            transaction_count_limit (int): The transaction count limit.

        Returns:
            TransactionHistory: The credit transactions for the user.
        """
        pass

    @abstractmethod
    async def spend_credits(
        self,
        entry: NodeExecutionEntry,
        data_size: float,
        run_time: float,
    ) -> int:
        """
        Spend the credits for the user based on the block usage.

        Args:
            entry (NodeExecutionEntry): The node execution identifiers & data.
            data_size (float): The size of the data being processed.
            run_time (float): The time taken to run the block.

        Returns:
            int: amount of credit spent
        """
        pass

    @abstractmethod
    async def top_up_credits(self, user_id: str, amount: int):
        """
        Top up the credits for the user immediately.
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/backend/backend/data/rabbitmq.py (Python)

Code before:
)

from backend.util.retry import conn_retry

load_dotenv()

HOST = os.getenv("RABBITMQ_HOST", "localhost")
PORT = int(os.getenv("RABBITMQ_PORT", "5672"))
USERNAME = os.getenv("RABBITMQ_DEFAULT_USER", "guest")
PASSWORD = os.getenv("RABBITMQ_DEFAULT_PASS", "guest")
VIRTUAL_HOST = os.getenv("RABBITMQ_VHOST", "/")

logger = logging.getLogger(__name__)

# Synchronous connections
connection: Optional[pika.BlockingConnection] = None
channel: Optional[BlockingChannel] = None

# Async connections
connection_async: Optional[AbstractConnection] = None
channel_async: Optional[AbstractChannel] = None


@conn_retry("RabbitMQ", "Acquiring connection")
def connect() -> pika.BlockingConnection:
    global connection, channel
    if connection and connection.is_open:
        return connection

    credentials = pika.PlainCredentials(USERNAME, PASSWORD)
    parameters = pika.ConnectionParameters(
        host=HOST,
        port=PORT,
        virtual_host=VIRTUAL_HOST,
        credentials=credentials,
        heartbeat=600,
        blocked_connection_timeout=300,
    )

    connection = pika.BlockingConnection(parameters)
    channel = connection.channel()
    channel.basic_qos(prefetch_count=1)
    return connection


@conn_retry("RabbitMQ", "Releasing connection")
def disconnect():
    global connection, channel
    if channel:
        if channel.is_open:
            channel.close()

Review comment (bug):
Can we avoid using globals please. It causes a lot of issues with state management and debugging.

It took ages to successful extract globals from autogpt classic, if you recall

Code after:
logger = logging.getLogger(__name__)


class ExchangeType(str, Enum):
    DIRECT = "direct"
    FANOUT = "fanout"
    TOPIC = "topic"
    HEADERS = "headers"


class Exchange(BaseModel):
    name: str
    type: ExchangeType
    durable: bool = True
    auto_delete: bool = False


class Queue(BaseModel):
    name: str
    durable: bool = True
    auto_delete: bool = False
    # Optional exchange binding configuration
    exchange: Optional[Exchange] = None
    routing_key: Optional[str] = None
    arguments: Optional[dict] = None


class RabbitMQConfig(BaseModel):
    """Configuration for a RabbitMQ service instance"""

    vhost: str = "/"
    exchanges: list[Exchange]
    queues: list[Queue]


class RabbitMQBase(ABC):
    """Base class for RabbitMQ connections with shared configuration"""

    def __init__(self, config: RabbitMQConfig):
        settings = Settings()
        self.host = settings.config.rabbitmq_host
        self.port = settings.config.rabbitmq_port
        self.username = settings.secrets.rabbitmq_default_user
        self.password = settings.secrets.rabbitmq_default_pass
        self.config = config

        self._connection = None
        self._channel = None

    @property
    def is_connected(self) -> bool:
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/backend/backend/data/rabbitmq.py (Python)

Code before:

class Exchange(BaseModel):
    name: str
    type: ExchangeType
    durable: bool = True
    auto_delete: bool = False


class Queue(BaseModel):
    name: str
    durable: bool = True
    auto_delete: bool = False
    # Optional exchange binding configuration
    exchange: Optional[Exchange] = None
    routing_key: Optional[str] = None
    arguments: Optional[dict] = None


class RabbitMQConfig(BaseModel):
    """Configuration for a RabbitMQ service instance"""

    vhost: str = "/"
    exchanges: list[Exchange]
    queues: list[Queue]


class RabbitMQBase(ABC):
    """Base class for RabbitMQ connections with shared configuration"""

    def __init__(self, config: RabbitMQConfig):
        settings = Settings()
        self.host = settings.config.rabbitmq_host
        self.port = settings.config.rabbitmq_port
        self.username = settings.secrets.rabbitmq_default_user
        self.password = settings.secrets.rabbitmq_default_pass
        self.config = config

        self._connection = None
        self._channel = None

    @property
    def is_connected(self) -> bool:
        """Check if we have a valid connection"""
        return bool(self._connection)

    @property
    def is_ready(self) -> bool:
        """Check if we have a valid channel"""
        return bool(self.is_connected and self._channel)

    @abstractmethod

Review comment (suggestion):
Expect this to change a bit as I get used to working with it. I'm thinking de-duplicating exchanges and queues is probably going to happen

Code after:
    HEADERS = "headers"


class Exchange(BaseModel):
    name: str
    type: ExchangeType
    durable: bool = True
    auto_delete: bool = False


class Queue(BaseModel):
    name: str
    durable: bool = True
    auto_delete: bool = False
    # Optional exchange binding configuration
    exchange: Optional[Exchange] = None
    routing_key: Optional[str] = None
    arguments: Optional[dict] = None


class RabbitMQConfig(BaseModel):
    """Configuration for a RabbitMQ service instance"""

    vhost: str = "/"
    exchanges: list[Exchange]
    queues: list[Queue]


class RabbitMQBase(ABC):
    """Base class for RabbitMQ connections with shared configuration"""

    def __init__(self, config: RabbitMQConfig):
        settings = Settings()
        self.host = settings.config.rabbitmq_host
        self.port = settings.config.rabbitmq_port
        self.username = settings.secrets.rabbitmq_default_user
        self.password = settings.secrets.rabbitmq_default_pass
        self.config = config

        self._connection = None
        self._channel = None

    @property
    def is_connected(self) -> bool:
        """Check if we have a valid connection"""
        return bool(self._connection)

    @property
    def is_ready(self) -> bool:
        """Check if we have a valid channel"""
        return bool(self.is_connected and self._channel)
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/frontend/src/components/agptui/delete-confirm-dialog.tsx (TypeScript)

Code before:
}: {
  entityType: string;
  entityName?: string;
  open: boolean;
  onOpenChange: (open: boolean) => void;
  onDoDelete: () => void;
  isIrreversible?: boolean;
  className?: string;
}): React.ReactNode {
  const displayType = entityType
    .split(" ")
    .map((word) => word.charAt(0).toUpperCase() + word.slice(1))
    .join(" ");
  return (
    <Dialog open={open} onOpenChange={onOpenChange}>
      <DialogContent className={className}>
        <DialogHeader>
          <DialogTitle>
            Delete {displayType} {entityName && `"${entityName}"`}
          </DialogTitle>
          <DialogDescription>
            Are you sure you want to delete this {entityType}?
            {isIrreversible && (
              <>
                <br /> This action cannot be undone.
              </>
            )}
          </DialogDescription>
        </DialogHeader>
        <DialogFooter>
          <Button onClick={() => onOpenChange(false)}>Cancel</Button>
          <Button variant="destructive" onClick={onDoDelete}>
            Delete
          </Button>
        </DialogFooter>
      </DialogContent>
    </Dialog>
  );
}


Review comment (suggestion):
```suggestion
              <b>
                <br /> This action cannot be undone.
              <b />
```

Code after:
}: {
  entityType: string;
  entityName?: string;
  open: boolean;
  onOpenChange: (open: boolean) => void;
  onDoDelete: () => void;
  isIrreversible?: boolean;
  className?: string;
}): React.ReactNode {
  const displayType = entityType
    .split(" ")
    .map((word) => word.charAt(0).toUpperCase() + word.slice(1))
    .join(" ");
  return (
    <Dialog open={open} onOpenChange={onOpenChange}>
      <DialogContent className={className}>
        <DialogHeader>
          <DialogTitle>
            Delete {displayType} {entityName && `"${entityName}"`}
          </DialogTitle>
          <DialogDescription>
            Are you sure you want to delete this {entityType}?
            {isIrreversible && (
              <b>
                <br /> This action cannot be undone.
              </b>
            )}
          </DialogDescription>
        </DialogHeader>
        <DialogFooter>
          <Button onClick={() => onOpenChange(false)}>Cancel</Button>
          <Button variant="destructive" onClick={onDoDelete}>
            Delete
          </Button>
        </DialogFooter>
      </DialogContent>
    </Dialog>
  );
}