ronantakizawa/codereview-bench huggingface dataset:
Repository: VinciGit00/Scrapegraph-ai
File: scrapegraphai/nodes/fetch_screen_node.py (Python)

Code before:
    def execute(self, state: dict) -> dict:
        """
        Captures screenshots from the input URL and stores them in the state dictionary as bytes.
        """

        screenshots = []

        with sync_playwright() as p:
            browser = p.chromium.launch()
            page = browser.new_page()
            page.goto(self.url)

            viewport_height = page.viewport_size["height"]

            screenshot_counter = 1

            screenshot_data_list = []

            def capture_screenshot(scroll_position, counter):
                page.evaluate(f"window.scrollTo(0, {scroll_position});")
                screenshot_data = page.screenshot()
                screenshot_data_list.append(screenshot_data)

            capture_screenshot(0, screenshot_counter)
            screenshot_counter += 1
            capture_screenshot(viewport_height, screenshot_counter)

            browser.close()

        for screenshot_data in screenshot_data_list:
            screenshots.append(screenshot_data)

        state["link"] = self.url
        state['screenshots'] = screenshots

        return state


Review comment (bug):
infinite-scrolling web pages (with dynamic JS rendering) may have undefined behavior with the viewport height, as that is bound to change as well as you scroll through the page in the browser.

looks fine for now, but I would maybe add a disclaimer in the docs saying screenshot capturing might not work as well for those; collecting only two screenshots is fine as well, but that number might better be a function of the viewport height, so that we don't miss parts of the page content in case a page is very very long (although not infinite-scrolling)

Code after:

    def execute(self, state: dict) -> dict:
        """
        Captures screenshots from the input URL and stores them in the state dictionary as bytes.
        """
        self.logger.info(f"--- Executing {self.node_name} Node ---")

        with sync_playwright() as p:
            browser = p.chromium.launch()
            page = browser.new_page()
            page.goto(self.url)

            viewport_height = page.viewport_size["height"]

            screenshot_counter = 1

            screenshot_data_list = []

            def capture_screenshot(scroll_position, counter):
                page.evaluate(f"window.scrollTo(0, {scroll_position});")
                screenshot_data = page.screenshot()
                screenshot_data_list.append(screenshot_data)

            capture_screenshot(0, screenshot_counter)
            screenshot_counter += 1
            capture_screenshot(viewport_height, screenshot_counter)

            browser.close()

        state["link"] = self.url
        state['screenshots'] = screenshot_data_list

        return state
Repository: VinciGit00/Scrapegraph-ai
File: scrapegraphai/nodes/generate_answer_from_image_node.py (Python)

Code before:

        api_key = self.node_config.get("config", {}).get("llm", {}).get("api_key", "")

        async with aiohttp.ClientSession() as session:
            tasks = [
                self.process_image(session, api_key, image_data,
                                   state.get("user_prompt", "Extract information from the image"))
                for image_data in images
            ]

            analyses = await asyncio.gather(*tasks)

        consolidated_analysis = " ".join(analyses)

        state['answer'] = {
            "consolidated_analysis": consolidated_analysis
        }

        return state

    def execute(self, state: dict) -> dict:
        """
        Wrapper to run the asynchronous execute_async function in a synchronous context.
        """
        return asyncio.run(self.execute_async(state))


Review comment (suggestion):
wrap the function with a safer guard on the running event loop, e.g.:

```python
def execute(self, state: dict) -> dict:
    """
    Wrapper to run the asynchronous execute_async function in a synchronous context.
    """
    try:
        eventloop = asyncio.get_event_loop()
    except RuntimeError:
        eventloop = None

    if eventloop and eventloop.is_running():
        state = eventloop.run_until_complete(self.execute_async(state))
    else:
        state = asyncio.run(self.execute_async(state))

    return state
```

Code after:

        if self.node_config["config"]["llm"]["model"] not in supported_models:
            raise ValueError(f"""Model '{self.node_config['config']['llm']['model']}'
                             is not supported. Supported models are: 
                             {', '.join(supported_models)}.""")

        api_key = self.node_config.get("config", {}).get("llm", {}).get("api_key", "")

        async with aiohttp.ClientSession() as session:
            tasks = [
                self.process_image(session, api_key, image_data,
                                   state.get("user_prompt", "Extract information from the image"))
                for image_data in images
            ]

            analyses = await asyncio.gather(*tasks)

        consolidated_analysis = " ".join(analyses)

        state['answer'] = {
            "consolidated_analysis": consolidated_analysis
        }

        return state

    def execute(self, state: dict) -> dict:
        """
        Wrapper to run the asynchronous execute_async function in a synchronous context.
        """
        try:
            eventloop = asyncio.get_event_loop()
        except RuntimeError:
            eventloop = None

        if eventloop and eventloop.is_running():
            task = eventloop.create_task(self.execute_async(state))
            state = eventloop.run_until_complete(asyncio.gather(task))[0]
        else:
            state = asyncio.run(self.execute_async(state))

        return state
Repository: Yidadaa/ChatGPT-Next-Web
File: app/constant.ts (TypeScript)

Code before:
  ExampleEndpoint: "https://{resource-url}/openai/deployments/{deploy-id}",
};

export const Google = {
  ExampleEndpoint: "https://generativelanguage.googleapis.com/",
  ChatPath: "v1beta/models/gemini-pro:generateContent",

  // /api/openai/v1/chat/completions
};

export const DEFAULT_INPUT_TEMPLATE = `{{input}}`; // input / time / model / lang
export const DEFAULT_SYSTEM_TEMPLATE = `
You are ChatGPT, a large language model trained by OpenAI.
Knowledge cutoff: {{cutoff}}
Current model: {{model}}
Current time: {{time}}
Latex inline: $x^2$ 
Latex block: $$e=mc^2$$
`;

export const SUMMARIZE_MODEL = "gpt-3.5-turbo";

export const KnowledgeCutOffDate: Record<string, string> = {
  default: "2021-09",
  "gpt-4-1106-preview": "2023-04",
  "gpt-4-vision-preview": "2023-04",
};

export const DEFAULT_MODELS = [
  {
    name: "gpt-4",
    available: true,
    provider: {
      id: "openai",
      providerName: "OpenAI",
      providerType: "openai",
    },
  },
  {
    name: "gpt-4-0314",
    available: true,
    provider: {
      id: "openai",
      providerName: "OpenAI",
      providerType: "openai",
    },
  },
  {
    name: "gpt-4-0613",
    available: true,
    provider: {

Review comment (suggestion):
```suggestion
  "gpt-4-0125-preview": "2023-04",
  "gpt-4-turbo-preview": "2023-04",
```

add more model

![image](https://github.com/ChatGPTNextWeb/ChatGPT-Next-Web/assets/17626300/0b58010f-804c-4b2d-aa25-e0774beba3a4)

> [!NOTE]  
> To be honest, there is only a minor difference regarding the new models.


Code after:
  ExampleEndpoint: "https://{resource-url}/openai/deployments/{deploy-id}",
};

export const Google = {
  ExampleEndpoint: "https://generativelanguage.googleapis.com/",
  ChatPath: "v1beta/models/gemini-pro:generateContent",

  // /api/openai/v1/chat/completions
};

export const DEFAULT_INPUT_TEMPLATE = `{{input}}`; // input / time / model / lang
export const DEFAULT_SYSTEM_TEMPLATE = `
You are ChatGPT, a large language model trained by OpenAI.
Knowledge cutoff: {{cutoff}}
Current model: {{model}}
Current time: {{time}}
Latex inline: $x^2$ 
Latex block: $$e=mc^2$$
`;

export const SUMMARIZE_MODEL = "gpt-3.5-turbo";

export const KnowledgeCutOffDate: Record<string, string> = {
  default: "2021-09",
  "gpt-4-1106-preview": "2023-04",
  "gpt-4-0125-preview": "2023-04",
  "gpt-4-vision-preview": "2023-04",
};

export const DEFAULT_MODELS = [
  {
    name: "gpt-4",
    available: true,
    provider: {
      id: "openai",
      providerName: "OpenAI",
      providerType: "openai",
    },
  },
  {
    name: "gpt-4-0314",
    available: true,
    provider: {
      id: "openai",
      providerName: "OpenAI",
      providerType: "openai",
    },
  },
  {
    name: "gpt-4-0613",
    available: true,
Repository: Yidadaa/ChatGPT-Next-Web
File: app/api/upstash/[action]/[...key]/route.ts (TypeScript)

Code before:
import { NextRequest, NextResponse } from "next/server";

async function handle(
  req: NextRequest,
  { params }: { params: { action: string; key: string[] } },
) {
  const requestUrl = new URL(req.url);
  const endpoint = requestUrl.searchParams.get("endpoint");

  if (req.method === "OPTIONS") {
    return NextResponse.json({ body: "OK" }, { status: 200 });
  }
  const [action, ...key] = params.key;
  // only allow to request to *.upstash.io
  if (!endpoint || !endpoint.endsWith("upstash.io")) {
    return NextResponse.json(
      {
        error: true,
        msg: "you are not allowed to request " + params.key.join("/"),
      },
      {
        status: 403,
      },
    );
  }

  // only allow upstash get and set method
  if (action !== "get" && action !== "set") {
    return NextResponse.json(
      {
        error: true,
        msg: "you are not allowed to request " + params.action,
      },
      {
        status: 403,
      },
    );
  }

  const [protocol, ...subpath] = params.key;

Review comment (suggestion):
is string `endsWith` enough here? It seems still possible to craft a endpoint like `https://arbitrary.url.i.want/path?dummy=upstash.io` and bypass.

What about 

```diff
try {
  if ( ! (new URL(endpoint).hostname.endswith('.upstash.io')   )  {
   throw 403
 }
} catch (e) {
  throw 400
}

Code after:
import { NextRequest, NextResponse } from "next/server";

async function handle(
  req: NextRequest,
  { params }: { params: { action: string; key: string[] } },
) {
  const requestUrl = new URL(req.url);
  const endpoint = requestUrl.searchParams.get("endpoint");

  if (req.method === "OPTIONS") {
    return NextResponse.json({ body: "OK" }, { status: 200 });
  }
  const [...key] = params.key;
  // only allow to request to *.upstash.io
  if (!endpoint || !new URL(endpoint).hostname.endsWith(".upstash.io")) {
    return NextResponse.json(
      {
        error: true,
        msg: "you are not allowed to request " + params.key.join("/"),
      },
      {
        status: 403,
      },
    );
  }

  // only allow upstash get and set method
  if (params.action !== "get" && params.action !== "set") {
    console.log("[Upstash Route] forbidden action ", params.action);
    return NextResponse.json(
      {
        error: true,
        msg: "you are not allowed to request " + params.action,
      },
      {
        status: 403,
      },
    );
  }
Repository: Yidadaa/ChatGPT-Next-Web
File: app/utils/model.ts (TypeScript)

Code before:
            customProviderName || customModelName,
          );
          // swap name and displayName for bytedance
          if (displayName && provider.providerName == "ByteDance") {
            [customModelName, displayName] = [displayName, customModelName];
          }
          modelTable[`${customModelName}@${provider?.id}`] = {
            name: customModelName,
            displayName: displayName || customModelName,
            available,
            provider, // Use optional chaining
          };
        }
      }
    });

  return modelTable;
}

export function collectModelTableWithDefaultModel(
  models: readonly LLMModel[],
  customModels: string,
  defaultModel: string,
) {
  let modelTable = collectModelTable(models, customModels);
  if (defaultModel && defaultModel !== "" && defaultModel in modelTable) {
    modelTable[defaultModel] = {
      ...modelTable[defaultModel],
      available: true,
      isDefault: true,
    };
  }
  return modelTable;
}

/**
 * Generate full model table.
 */
export function collectModels(
  models: readonly LLMModel[],
  customModels: string,
) {
  const modelTable = collectModelTable(models, customModels);
  const allModels = Object.values(modelTable);

  return allModels;
}

export function collectModelsWithDefaultModel(
  models: readonly LLMModel[],
  customModels: string,

Review comment (suggestion):
please see L82:
```
modelTable[`${customModelName}@${provider?.id}`] = {...}
```

Code after:
            customProviderName || customModelName,
          );
          // swap name and displayName for bytedance
          if (displayName && provider.providerName == "ByteDance") {
            [customModelName, displayName] = [displayName, customModelName];
          }
          modelTable[`${customModelName}@${provider?.id}`] = {
            name: customModelName,
            displayName: displayName || customModelName,
            available,
            provider, // Use optional chaining
          };
        }
      }
    });

  return modelTable;
}

export function collectModelTableWithDefaultModel(
  models: readonly LLMModel[],
  customModels: string,
  defaultModel: string,
) {
  let modelTable = collectModelTable(models, customModels);
  if (defaultModel && defaultModel !== "") {
    if (defaultModel.includes('@')) {
      if (defaultModel in modelTable) {
        modelTable[defaultModel].isDefault = true;
      }
    } else {
      for (const key of Object.keys(modelTable)) {
        if (modelTable[key].available && key.split('@').shift() == defaultModel) {
          modelTable[key].isDefault = true;
          break;
        }
      }
    }
  }
  return modelTable;
}

/**
 * Generate full model table.
 */
export function collectModels(
  models: readonly LLMModel[],
  customModels: string,
) {
  const modelTable = collectModelTable(models, customModels);
  const allModels = Object.values(modelTable);
Repository: Yidadaa/ChatGPT-Next-Web
File: app/components/ui-lib.tsx (TypeScript)

Code before:

  root.render(<Toast content={content} action={action} onClose={close} />);
}

export type InputProps = React.HTMLProps<HTMLTextAreaElement> & {
  autoHeight?: boolean;
  rows?: number;
};

export function Input(props: InputProps) {
  return (
    <textarea
      {...props}
      className={`${styles["input"]} ${props.className}`}
    ></textarea>
  );
}

// 定义一个接口 AriaProps，包含一个可选的 aria 属性
interface AriaProps {
  aria?: string;
}

// 定义一个接口 PasswordInputProps，继承自 HTMLProps<HTMLInputElement> 和 AriaProps
// 用于描述密码输入框组件的属性
interface PasswordInputProps extends HTMLProps<HTMLInputElement>, AriaProps {}

export function PasswordInput(props: PasswordInputProps) {
  const [visible, setVisible] = useState(false);
  function changeVisibility() {
    setVisible(!visible);
  }

  return (
    <div className={"password-input-container"}>
      <IconButton
        aria={props.aria}
        icon={visible ? <EyeIcon /> : <EyeOffIcon />}
        onClick={changeVisibility}
        className={"password-eye"}
      />
      <input
        {...props}
        type={visible ? "text" : "password"}
        className={"password-input"}
      />
    </div>
  );
}

export function Select(

Review comment (suggestion):
type Props = HTMLProps<HTMLInputElement> &  {
  aria?: string;
}
是不是就可以了？

或者
interface PasswordInputProps extends HTMLProps<HTMLInputElement>{
  aria?: string;
}

Code after:

  root.render(<Toast content={content} action={action} onClose={close} />);
}

export type InputProps = React.HTMLProps<HTMLTextAreaElement> & {
  autoHeight?: boolean;
  rows?: number;
};

export function Input(props: InputProps) {
  return (
    <textarea
      {...props}
      className={`${styles["input"]} ${props.className}`}
    ></textarea>
  );
}

export function PasswordInput(
  props: HTMLProps<HTMLInputElement> & { aria?: string },
) {
  const [visible, setVisible] = useState(false);
  function changeVisibility() {
    setVisible(!visible);
  }

  return (
    <div className={"password-input-container"}>
      <IconButton
        aria={props.aria}
        icon={visible ? <EyeIcon /> : <EyeOffIcon />}
        onClick={changeVisibility}
        className={"password-eye"}
      />
      <input
        {...props}
        type={visible ? "text" : "password"}
        className={"password-input"}
      />
    </div>
  );
}

export function Select(
  props: React.DetailedHTMLProps<
    React.SelectHTMLAttributes<HTMLSelectElement>,
    HTMLSelectElement
  >,
) {
  const { className, children, ...otherProps } = props;
  return (
Repository: Yidadaa/ChatGPT-Next-Web
File: app/components/markdown.tsx (TypeScript)

Code before:
        "tex",
        "latex",
      ];
      codeElements.forEach((codeElement) => {
        let languageClass = codeElement.className.match(/language-(\w+)/);
        let name = languageClass ? languageClass[1] : "";
        if (wrapLanguages.includes(name)) {
          codeElement.style.whiteSpace = "pre-wrap";
        }
      });
    }
  }, []);

  const [collapsed, setCollapsed] = useState(true);
  const [showToggle, setShowToggle] = useState(false);

  useEffect(() => {
    if (ref.current) {
      const codeHeight = ref.current.scrollHeight;
      setShowToggle(codeHeight > 400);
      ref.current.scrollTop = ref.current.scrollHeight;
    }
  }, [props.children]);

  const toggleCollapsed = () => {
    setCollapsed(!collapsed);
  };

  return (
    <>
      <div style={{ position: "relative" }}>
        <pre
          ref={ref}
          style={{
            maxHeight: collapsed ? "400px" : "none",
            overflow: "hidden",
          }}
        >
          <span
            className="copy-code-button"
            onClick={() => {
              if (ref.current) {
                const code = ref.current.innerText;
                copyToClipboard(code);
              }
            }}
          ></span>
          {props.children}
          {showToggle && collapsed && (
            <div
              className="show-hide-button"

Review comment (suggestion):
```typescript
setCollapsed(collapsed=>!collapsed)
```

Code after:
        "tex",
        "latex",
      ];
      codeElements.forEach((codeElement) => {
        let languageClass = codeElement.className.match(/language-(\w+)/);
        let name = languageClass ? languageClass[1] : "";
        if (wrapLanguages.includes(name)) {
          codeElement.style.whiteSpace = "pre-wrap";
        }
      });
    }
  }, []);

  return (
    <>
      <pre ref={ref}>
        <span
          className="copy-code-button"
          onClick={() => {
            if (ref.current) {
              const code = ref.current.innerText;
              copyToClipboard(code);
            }
          }}
        ></span>
        {props.children}
      </pre>
      {mermaidCode.length > 0 && (
        <Mermaid code={mermaidCode} key={mermaidCode} />
      )}
      {htmlCode.length > 0 && enableArtifacts && (
        <FullScreen className="no-dark html" right={70}>
          <ArtifactsShareButton
            style={{ position: "absolute", right: 20, top: 10 }}
            getCode={() => htmlCode}
          />
          <HTMLPreview
            code={htmlCode}
            autoHeight={!document.fullscreenElement}
            height={!document.fullscreenElement ? 600 : height}
          />
        </FullScreen>
      )}
    </>
  );
}

function CustomCode(props: { children: any }) {
  const ref = useRef<HTMLPreElement>(null);
  const [collapsed, setCollapsed] = useState(true);
  const [showToggle, setShowToggle] = useState(false);
Repository: aipotheosis-labs/aci
File: aipolabs/common/schemas/agent.py (Python)

Code before:


class AgentPublic(BaseModel):
    id: UUID
    project_id: UUID
    name: str
    description: str
    excluded_apps: list[UUID] = []
    excluded_functions: list[UUID] = []
    custom_instructions: dict[UUID, str] = {}

    created_at: datetime
    updated_at: datetime

    api_keys: list[APIKeyPublic]

    model_config = ConfigDict(from_attributes=True)


class CustomInstructionsCreate(BaseModel):
    """
    Custom instructions for a agent to follow for an app within a project
    """

    app_id: UUID
    instructions: str


Review comment (suggestion):
you do need some restrictions on instructions str, it serves as validation on client side (e.g., no empty str) and also safeguard on database. If you are not sure what value makes sense yet, at least leave a TODO note.

and something to think about (relevant to the comment below regarding the agent endpoint): when client creating the agent,  should "instructions" be optional? if optional and not provided by users, what default value should it has

Code after:
        raise ValueError("Instructions cannot be empty strings")
    return v


ValidInstruction = Annotated[str, BeforeValidator(validate_instruction)]
CustomInstructions = dict[UUID, ValidInstruction]


class AgentCreate(BaseModel):
    name: str
    description: str
    excluded_apps: list[UUID] = []
    excluded_functions: list[UUID] = []
    custom_instructions: CustomInstructions = Field(default_factory=dict)
    model_config = ConfigDict(json_encoders={UUID: str})


class AgentUpdate(BaseModel):
    name: str | None = None
    description: str | None = None
    excluded_apps: list[UUID] | None = None
    excluded_functions: list[UUID] | None = None
    custom_instructions: CustomInstructions = Field(default_factory=dict)
    model_config = ConfigDict(json_encoders={UUID: str})


class AgentPublic(BaseModel):
    id: UUID
    project_id: UUID
    name: str
    description: str
    excluded_apps: list[UUID] = []
    excluded_functions: list[UUID] = []
    custom_instructions: CustomInstructions = Field(default_factory=dict)

    created_at: datetime
    updated_at: datetime

    api_keys: list[APIKeyPublic]

    model_config = ConfigDict(from_attributes=True, json_encoders={UUID: str})