ronantakizawa/codereview-bench huggingface dataset:
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/backend/backend/blocks/iteration.py (Python)

Code before:
from typing import Any

from backend.data.block import Block, BlockCategory, BlockOutput, BlockSchema
from backend.data.model import SchemaField


class ListIteratorBlock(Block):
    class Input(BlockSchema):
        items: list | dict = SchemaField(
            description="The list or dictionary of items to iterate over",
            placeholder="[1, 2, 3, 4, 5] or {'key1': 'value1', 'key2': 'value2'}",
        )

    class Output(BlockSchema):
        item: Any = SchemaField(description="The current item in the iteration")
        key: Any = SchemaField(
            description="The key or index of the current item in the iteration",
        )

    def __init__(self):
        super().__init__(
            id="f8e7d6c5-b4a3-2c1d-0e9f-8g7h6i5j4k3l",
            input_schema=ListIteratorBlock.Input,
            output_schema=ListIteratorBlock.Output,
            categories={BlockCategory.LOGIC},
            description="Iterates over a list or dictionary and outputs each item.",
            test_input={"items": [1, 2, 3, {"key1": "value1", "key2": "value2"}]},
            test_output=[
                ("item", 1),
                ("key", 0),
                ("item", 2),
                ("key", 1),

Review comment (suggestion):
Rename block to just iterator maybe?  Iterator is a very programming term but this now also does dicts 

Code after:
from typing import Any

from backend.data.block import Block, BlockCategory, BlockOutput, BlockSchema
from backend.data.model import SchemaField


class StepThroughItemsBlock(Block):
    class Input(BlockSchema):
        items: list | dict = SchemaField(
            description="The list or dictionary of items to iterate over",
            placeholder="[1, 2, 3, 4, 5] or {'key1': 'value1', 'key2': 'value2'}",
        )

    class Output(BlockSchema):
        item: Any = SchemaField(description="The current item in the iteration")
        key: Any = SchemaField(
            description="The key or index of the current item in the iteration",
        )

    def __init__(self):
        super().__init__(
            id="f8e7d6c5-b4a3-2c1d-0e9f-8g7h6i5j4k3l",
            input_schema=StepThroughItemsBlock.Input,
            output_schema=StepThroughItemsBlock.Output,
            categories={BlockCategory.LOGIC},
            description="Iterates over a list or dictionary and outputs each item.",
            test_input={"items": [1, 2, 3, {"key1": "value1", "key2": "value2"}]},
            test_output=[
                ("item", 1),
                ("key", 0),
                ("item", 2),
                ("key", 1),
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/backend/backend/integrations/oauth/__init__.py (Python)

Code before:
from .base import BaseOAuthHandler
from .github import GitHubOAuthHandler
from .google import GoogleOAuthHandler
from .notion import NotionOAuthHandler

# --8<-- [start:HANDLERS_BY_NAMEExample]
HANDLERS_BY_NAME: dict[str, type[BaseOAuthHandler]] = {
    handler.PROVIDER_NAME: handler
    for handler in [
        GitHubOAuthHandler,
        GoogleOAuthHandler,
        NotionOAuthHandler,
    ]
}
# --8<-- [end: HANDLERS_BY_NAMEExample]

__all__ = ["HANDLERS_BY_NAME"]


Review comment (security):
nit: inconsistent; `GoogleOAuthIsConfigured` is lowercase but const is uppercase, here both comment and const are uppercase and `Example` is added. Sometimes there's space after `start: ` and sometimes not; I assume and hope that won't break docs.

Code after:
from .base import BaseOAuthHandler
from .github import GitHubOAuthHandler
from .google import GoogleOAuthHandler
from .notion import NotionOAuthHandler

# --8<-- [start:HANDLERS_BY_NAMEExample]
HANDLERS_BY_NAME: dict[str, type[BaseOAuthHandler]] = {
    handler.PROVIDER_NAME: handler
    for handler in [
        GitHubOAuthHandler,
        GoogleOAuthHandler,
        NotionOAuthHandler,
    ]
}
# --8<-- [end:HANDLERS_BY_NAMEExample]

__all__ = ["HANDLERS_BY_NAME"]
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/backend/backend/data/graph.py (Python)

Code before:

    graphs = await AgentGraph.prisma().find_many(
        where=where_clause,
        distinct=["id"],
        order={"version": "desc"},
        include=(
            AgentGraphInclude(
                AgentGraphExecution={"include": {"AgentNodeExecutions": True}}
            )
            if include_executions
            else None
        ),
    )

    if not graphs:
        return []

    return [GraphMeta.from_db(graph) for graph in graphs]


async def get_graph(
    graph_id: str,
    version: int | None = None,
    template: bool = False,
    user_id: str | None = None,
    hide_secrets: bool = False,
) -> Graph | None:
    """
    Retrieves a graph from the DB.
    Defaults to the version with `is_active` if `version` is not passed,
    or the latest version with `is_template` if `template=True`.

    Returns `None` if the record is not found.
    """
    where_clause: prisma.types.AgentGraphWhereInput = {
        "id": graph_id,
        "isTemplate": template,
    }
    if version is not None:
        where_clause["version"] = version
    elif not template:
        where_clause["isActive"] = True

    if user_id and not template:
        where_clause["userId"] = user_id

    graph = await AgentGraph.prisma().find_first(
        where=where_clause,
        include=AGENT_GRAPH_INCLUDE,
        order={"version": "desc"},
    )

Review comment (security):
nit: rename this `hide_credentials` or `omit_credentials` so we can keep using it after moving all legacy block secrets over to the credentials system

Code after:

    graphs = await AgentGraph.prisma().find_many(
        where=where_clause,
        distinct=["id"],
        order={"version": "desc"},
        include=(
            AgentGraphInclude(
                AgentGraphExecution={"include": {"AgentNodeExecutions": True}}
            )
            if include_executions
            else None
        ),
    )

    if not graphs:
        return []

    return [GraphMeta.from_db(graph) for graph in graphs]


async def get_graph(
    graph_id: str,
    version: int | None = None,
    template: bool = False,
    user_id: str | None = None,
    hide_credentials: bool = False,
) -> Graph | None:
    """
    Retrieves a graph from the DB.
    Defaults to the version with `is_active` if `version` is not passed,
    or the latest version with `is_template` if `template=True`.

    Returns `None` if the record is not found.
    """
    where_clause: prisma.types.AgentGraphWhereInput = {
        "id": graph_id,
        "isTemplate": template,
    }
    if version is not None:
        where_clause["version"] = version
    elif not template:
        where_clause["isActive"] = True

    if user_id and not template:
        where_clause["userId"] = user_id

    graph = await AgentGraph.prisma().find_first(
        where=where_clause,
        include=AGENT_GRAPH_INCLUDE,
        order={"version": "desc"},
    )
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/backend/backend/blocks/basic.py (Python)

Code before:
import re
from typing import Any, List

from jinja2 import BaseLoader, Environment

from backend.data.block import Block, BlockCategory, BlockOutput, BlockSchema, BlockType
from backend.data.model import SchemaField
from backend.data.model import SchemaField as Field
from backend.util.mock import MockObject

jinja = Environment(loader=BaseLoader())


class StoreValueBlock(Block):
    """
    This block allows you to provide a constant value as a block, in a stateless manner.
    The common use-case is simply pass the `input` data, it will `output` the same data.
    The block output will be static, the output can be consumed multiple times.
    """

    class Input(BlockSchema):
        input: Any = Field(
            description="Trigger the block to produce the output. "
            "The value is only used when `data` is None."
        )
        data: Any = Field(
            description="The constant data to be retained in the block. "
            "This value is passed as `output`.",
            default=None,
        )

    class Output(BlockSchema):
        output: Any = Field(description="The stored data retained in the block.")

Review comment (suggestion):
I'd prefer having a bigger diff here doing a find and replace on Field with SchemaField. 

Overriding Field will probably lead to confusion down the line

Code after:
import re
from typing import Any, List

from jinja2 import BaseLoader, Environment

from backend.data.block import Block, BlockCategory, BlockOutput, BlockSchema, BlockType
from backend.data.model import SchemaField
from backend.util.mock import MockObject

jinja = Environment(loader=BaseLoader())


class StoreValueBlock(Block):
    """
    This block allows you to provide a constant value as a block, in a stateless manner.
    The common use-case is simply pass the `input` data, it will `output` the same data.
    The block output will be static, the output can be consumed multiple times.
    """

    class Input(BlockSchema):
        input: Any = SchemaField(
            description="Trigger the block to produce the output. "
            "The value is only used when `data` is None."
        )
        data: Any = SchemaField(
            description="The constant data to be retained in the block. "
            "This value is passed as `output`.",
            default=None,
        )

    class Output(BlockSchema):
        output: Any = SchemaField(description="The stored data retained in the block.")
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/frontend/src/components/nav/MarketPopup.tsx (TypeScript)

Code before:
import { ButtonHTMLAttributes } from "react";
import React from "react";

interface MarketPopupProps extends ButtonHTMLAttributes<HTMLButtonElement> {
  marketplaceUrl?: string;
}

export default function MarketPopup({
  className = "",
  marketplaceUrl = (() => {
    if (process.env.APP_ENV === "prod") {
      return "https://production-marketplace-url.com";
    } else if (process.env.APP_ENV === "dev") {
      return "https://dev-builder.agpt.co/marketplace";
    } else {
      return "http://localhost:3000/marketplace";
    }
  })(),
  children,
  ...props
}: MarketPopupProps) {
  const openMarketplacePopup = () => {
    window.open(
      marketplaceUrl,
      "popupWindow",
      "width=600,height=400,toolbar=no,menubar=no,scrollbars=no",
    );
  };

  return (
    <button onClick={openMarketplacePopup} className={className} {...props}>
      {children}
    </button>
  );
}


Review comment (suggestion):
This isn't actually accessible on the frontend in testing :/ i think we will need a different way to handle this

Code after:
import { ButtonHTMLAttributes } from "react";
import React from "react";

interface MarketPopupProps extends ButtonHTMLAttributes<HTMLButtonElement> {
  marketplaceUrl?: string;
}

export default function MarketPopup({
  className = "",
  marketplaceUrl = (() => {
    if (process.env.NEXT_PUBLIC_APP_ENV === "prod") {
      return "https://production-marketplace-url.com";
    } else if (process.env.NEXT_PUBLIC_APP_ENV === "dev") {
      return "https://dev-builder.agpt.co/marketplace";
    } else {
      return "http://localhost:3000/marketplace";
    }
  })(),
  children,
  ...props
}: MarketPopupProps) {
  const openMarketplacePopup = () => {
    window.open(
      marketplaceUrl,
      "popupWindow",
      "width=600,height=400,toolbar=no,menubar=no,scrollbars=no",
    );
  };

  return (
    <button onClick={openMarketplacePopup} className={className} {...props}>
      {children}
    </button>
  );
}
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/frontend/src/components/monitor/FlowRunInfo.tsx (TypeScript)

Code before:
              <Button onClick={handleStopRun} variant="destructive">
                <IconSquare className="mr-2" /> Stop Run
              </Button>
            )}
            <Button onClick={() => setIsOutputOpen(true)} variant="outline">
              <ExitIcon className="mr-2" /> View Outputs
            </Button>
            <Link
              className={buttonVariants({ variant: "outline" })}
              href={`/build?flowID=${flow.id}`}
            >
              <Pencil2Icon className="mr-2" /> Edit Agent
            </Link>
          </div>
        </CardHeader>
        <CardContent>
          <p>
            <strong>Status:</strong>{" "}
            <FlowRunStatusBadge status={flowRun.status} />
          </p>
          <p>
            <strong>Started:</strong>{" "}
            {moment(flowRun.startTime).format("YYYY-MM-DD HH:mm:ss")}
          </p>
          <p>
            <strong>Finished:</strong>{" "}
            {moment(flowRun.endTime).format("YYYY-MM-DD HH:mm:ss")}
          </p>
          <p>
            <strong>Duration (run time):</strong> {flowRun.duration} (
            {flowRun.totalRunTime}) seconds
          </p>
          {/* <p><strong>Total cost:</strong> €1,23</p> */}
        </CardContent>
      </Card>
      <RunnerOutputUI
        isOpen={isOutputOpen}
        onClose={() => setIsOutputOpen(false)}
        blockOutputs={blockOutputs}
      />
    </>
  );
};
export default FlowRunInfo;


Review comment (refactor):
It's unrelated to your PR but I don't see the point of totalRunTime here, the maintainer of the system might care but why does the user care about this number? We might just remove this info altogether.

Code after:
              <ExitIcon className="mr-2" /> View Outputs
            </Button>
            <Link
              className={buttonVariants({ variant: "default" })}
              href={`/build?flowID=${flow.id}`}
            >
              <Pencil2Icon className="mr-2" /> Open in Builder
            </Link>
          </div>
        </CardHeader>
        <CardContent>
          <div>
            <strong>Status:</strong>{" "}
            <FlowRunStatusBadge status={flowRun.status} />
          </div>
          <p>
            <strong>Started:</strong>{" "}
            {moment(flowRun.startTime).format("YYYY-MM-DD HH:mm:ss")}
          </p>
          <p>
            <strong>Finished:</strong>{" "}
            {moment(flowRun.endTime).format("YYYY-MM-DD HH:mm:ss")}
          </p>
          <p>
            <strong>Duration (run time):</strong> {flowRun.duration} (
            {flowRun.totalRunTime}) seconds
          </p>
        </CardContent>
      </Card>
      <RunnerOutputUI
        isOpen={isOutputOpen}
        onClose={() => setIsOutputOpen(false)}
        blockOutputs={blockOutputs}
      />
    </>
  );
};

export default FlowRunInfo;
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/frontend/src/components/monitor/FlowRunInfo.tsx (TypeScript)

Code before:
            </Button>
            <Link
              className={buttonVariants({ variant: "outline" })}
              href={`/build?flowID=${flow.id}`}
            >
              <Pencil2Icon className="mr-2" /> Edit Agent
            </Link>
          </div>
        </CardHeader>
        <CardContent>
          <div>
            <strong>Status:</strong>{" "}
            <FlowRunStatusBadge status={flowRun.status} />
          </div>
          <p>
            <strong>Started:</strong>{" "}
            {moment(flowRun.startTime).format("YYYY-MM-DD HH:mm:ss")}
          </p>
          <p>
            <strong>Finished:</strong>{" "}
            {moment(flowRun.endTime).format("YYYY-MM-DD HH:mm:ss")}
          </p>
          <p>
            <strong>Duration (run time):</strong> {flowRun.duration}s
          </p>
          {/* <p><strong>Total cost:</strong> €1,23</p> */}
        </CardContent>
      </Card>
      <RunnerOutputUI
        isOpen={isOutputOpen}
        onClose={() => setIsOutputOpen(false)}
        blockOutputs={blockOutputs}
      />
    </>
  );
};
export default FlowRunInfo;


Review comment (refactor):
nit: I know this might not be your change, but can you remove this comment if you end up adding more commits on this PR ? :D

Code after:
            >
              <Pencil2Icon className="mr-2" /> Open in Builder
            </Link>
          </div>
        </CardHeader>
        <CardContent>
          <div>
            <strong>Status:</strong>{" "}
            <FlowRunStatusBadge status={flowRun.status} />
          </div>
          <p>
            <strong>Started:</strong>{" "}
            {moment(flowRun.startTime).format("YYYY-MM-DD HH:mm:ss")}
          </p>
          <p>
            <strong>Finished:</strong>{" "}
            {moment(flowRun.endTime).format("YYYY-MM-DD HH:mm:ss")}
          </p>
          <p>
            <strong>Duration (run time):</strong> {flowRun.duration} (
            {flowRun.totalRunTime}) seconds
          </p>
        </CardContent>
      </Card>
      <RunnerOutputUI
        isOpen={isOutputOpen}
        onClose={() => setIsOutputOpen(false)}
        blockOutputs={blockOutputs}
      />
    </>
  );
};

export default FlowRunInfo;
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/backend/backend/data/model.py (Python)

Code before:
    This is enforced by the `BlockSchema` base class.
    """
    # Convert provider list to enum for schema
    provider_list = [provider] if isinstance(provider, str) else provider

    json_extra = {
        k: v
        for k, v in {
            "credentials_provider": (
                provider if isinstance(provider, str) else list(provider)
            ),
            "credentials_scopes": list(required_scopes) or None,  # omit if empty
            "credentials_types": list(supported_credential_types),
            "discriminator": discriminator,
            "discriminator_mapping": discriminator_mapping,
        }.items()
        if v is not None
    }

    # Force provider to always be a string
    # Otherwise input provider will fail jsonschema validation
    # if multiple providers are provided
    json_extra.update(
        {"properties": {"provider": {"type": "string", "enum": provider_list}}}
    )

    return Field(
        title=title,
        description=description,
        json_schema_extra=json_extra,
        **kwargs,
    )


class ContributorDetails(BaseModel):
    name: str = Field(title="Name", description="The name of the contributor.")


Review comment (suggestion):
I think you can (and should) avoid this hack by changing the `provider` parameter to `provider: CP | list[CP]` and change the definition of `CP` back to `TypeVar("CP", bound=str)`

Code after:
    This is enforced by the `BlockSchema` base class.
    """
    json_extra = {
        k: v
        for k, v in {
            "credentials_provider": (
                [provider] if isinstance(provider, str) else provider
            ),
            "credentials_scopes": list(required_scopes) or None,  # omit if empty
            "credentials_types": list(supported_credential_types),
            "discriminator": discriminator,
            "discriminator_mapping": discriminator_mapping,
        }.items()
        if v is not None
    }

    return Field(
        title=title,
        description=description,
        json_schema_extra=json_extra,
        **kwargs,
    )


class ContributorDetails(BaseModel):
    name: str = Field(title="Name", description="The name of the contributor.")
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/frontend/src/components/integrations/credentials-input.tsx (TypeScript)

Code before:
  ollama: fallbackIcon,
  openai: fallbackIcon,
  openweathermap: fallbackIcon,
  pinecone: fallbackIcon,
  replicate: fallbackIcon,
  revid: fallbackIcon,
  unreal_speech: fallbackIcon,
};
// --8<-- [end:ProviderIconsEmbed]

export type OAuthPopupResultMessage = { message_type: "oauth_popup_result" } & (
  | {
      success: true;
      code: string;
      state: string;
    }
  | {
      success: false;
      message: string;
    }
);

export const CredentialsInput: FC<{
  className?: string;
  selectedCredentials?: CredentialsMetaInput;
  onSelectCredentials: (newValue: CredentialsMetaInput | undefined) => void;
}> = ({ className, selectedCredentials, onSelectCredentials }) => {
  const api = useMemo(() => new AutoGPTServerAPI(), []);
  const credentials = useCredentials();
  const [isAPICredentialsModalOpen, setAPICredentialsModalOpen] =
    useState(false);
  const [isOAuth2FlowInProgress, setOAuth2FlowInProgress] = useState(false);
  const [oAuthPopupController, setOAuthPopupController] =
    useState<AbortController | null>(null);
  const [oAuthError, setOAuthError] = useState<string | null>(null);

  if (!credentials || credentials.isLoading) {
    return null;
  }

  const {
    schema,
    provider,
    providerName,
    supportsApiKey,
    supportsOAuth2,
    savedApiKeys,
    savedOAuthCredentials,
    oAuthCallback,
  } = credentials;


Review comment (security):
```suggestion
  onSelectCredentials: (newValue?: CredentialsMetaInput) => void;
```

Code after:
  ollama: fallbackIcon,
  openai: fallbackIcon,
  openweathermap: fallbackIcon,
  pinecone: fallbackIcon,
  replicate: fallbackIcon,
  revid: fallbackIcon,
  unreal_speech: fallbackIcon,
};
// --8<-- [end:ProviderIconsEmbed]

export type OAuthPopupResultMessage = { message_type: "oauth_popup_result" } & (
  | {
      success: true;
      code: string;
      state: string;
    }
  | {
      success: false;
      message: string;
    }
);

export const CredentialsInput: FC<{
  className?: string;
  selectedCredentials?: CredentialsMetaInput;
  onSelectCredentials: (newValue?: CredentialsMetaInput) => void;
}> = ({ className, selectedCredentials, onSelectCredentials }) => {
  const api = useMemo(() => new AutoGPTServerAPI(), []);
  const credentials = useCredentials();
  const [isAPICredentialsModalOpen, setAPICredentialsModalOpen] =
    useState(false);
  const [isOAuth2FlowInProgress, setOAuth2FlowInProgress] = useState(false);
  const [oAuthPopupController, setOAuthPopupController] =
    useState<AbortController | null>(null);
  const [oAuthError, setOAuthError] = useState<string | null>(null);

  if (!credentials || credentials.isLoading) {
    return null;
  }

  const {
    schema,
    provider,
    providerName,
    supportsApiKey,
    supportsOAuth2,
    savedApiKeys,
    savedOAuthCredentials,
    oAuthCallback,
  } = credentials;