ronantakizawa/codereview-bench huggingface dataset:
Repository: puppeteer/puppeteer
File: test/src/mocha-utils.ts (TypeScript)

Code before:
  afterEach(async () => {
    if (state.context) {
      await state.context.close();
      state.context = undefined;
      state.page = undefined;
    }
  });
};

export const setupSeparateTestBrowserHooks = (
  launchOptions: Readonly<LaunchOptions>,
  options: {
    createContext?: boolean;
    createPage?: boolean;
  } = {},
): Awaited<ReturnType<typeof launch>> => {
  const {createContext = true, createPage = true} = options;

  const state: Awaited<ReturnType<typeof launch>> = {} as any;
  before(async function () {
    const {timeout, protocolTimeout} = adjustBrowserLaunchTimeout(this);
    const browserState = await launch(
      {
        ...launchOptions,
        timeout,
        protocolTimeout,
      },
      {
        after: 'all',
        ...options,
      },
    );
    // Trick to keep the correct reference
    const props = Object.entries(browserState).reduce((acc, entries) => {
      const [key, value] = entries;
      acc[key] = {
        value,
        writable: true,
      };
      return acc;
    }, {} as PropertyDescriptorMap);
    Object.defineProperties(state, props);
  });

  if (createContext) {
    beforeEach(async () => {
      state.context = await state.browser.createBrowserContext();
      if (createPage) {
        state.page = await state.context.newPage();
      }
    });

Review comment (suggestion):
```suggestion
        timeout,
        protocolTimeout,
        ...launchOptions,
```

Code after:
  afterEach(async () => {
    if (state.context) {
      await state.context.close();
      state.context = undefined;
      state.page = undefined;
    }
  });
};

export const setupSeparateTestBrowserHooks = (
  launchOptions: Readonly<LaunchOptions>,
  options: {
    createContext?: boolean;
    createPage?: boolean;
  } = {},
): Awaited<ReturnType<typeof launch>> => {
  const {createContext = true, createPage = true} = options;

  const state: Awaited<ReturnType<typeof launch>> = {} as any;
  before(async function () {
    const {timeout, protocolTimeout} = adjustBrowserLaunchTimeout(this);
    const browserState = await launch(
      {
        timeout,
        protocolTimeout,
        ...launchOptions,
      },
      {
        after: 'all',
        ...options,
      },
    );
    // Trick to keep the correct reference
    const props = Object.entries(browserState).reduce((acc, entries) => {
      const [key, value] = entries;
      acc[key] = {
        value,
        writable: true,
      };
      return acc;
    }, {} as PropertyDescriptorMap);
    Object.defineProperties(state, props);
  });

  if (createContext) {
    beforeEach(async () => {
      state.context = await state.browser.createBrowserContext();
      if (createPage) {
        state.page = await state.context.newPage();
      }
    });
Repository: puppeteer/puppeteer
File: test/src/touchscreen.spec.ts (TypeScript)

Code before:
          height: 1,
          altitudeAngle: 1.5707963267948966,
          azimuthAngle: 0,
          pressure: 0,
          pointerType: 'touch',
          twist: 0,
          tiltX: 0,
          tiltY: 0,
        },
        {
          type: 'touchend',
          changedTouches: [
            {
              clientX: 50,
              clientY: 10,
              radiusX: 0.5,
              radiusY: 0.5,
              force: 0.5,
            },
          ],
          activeTouches: [],
        },
      ]);
    });

    it.only('should work with three touches', async () => {
      const {page, server} = await getTestState();
      await page.goto(server.PREFIX + '/input/touchscreen.html');

      const touch1 = await page.touchscreen.touchStart(50, 50);
      await touch1.move(50, 100);
      await page.touchscreen.touchStart(20, 20);
      await touch1.end();
      const touch3 = await page.touchscreen.touchStart(20, 100);
      await touch3.move(60, 100);

      expect(
        await page.evaluate(async () => {
          // @ts-expect-error executed on the page
          return await globalThis.waitForEvents;
        }),
      ).toMatchObject([
        {
          type: 'pointerdown',
          x: 50,
          y: 50,
          width: 1,
          height: 1,
          altitudeAngle: 1.5707963267948966,
          azimuthAngle: 0,
          pressure: 0.5,

Review comment (suggestion):
```suggestion
    it('should work with three touches', async () => {
```

Code after:
          height: 1,
          altitudeAngle: 1.5707963267948966,
          azimuthAngle: 0,
          pressure: 0,
          pointerType: 'touch',
          twist: 0,
          tiltX: 0,
          tiltY: 0,
        },
        {
          type: 'touchend',
          changedTouches: [
            {
              clientX: 50,
              clientY: 10,
              radiusX: 0.5,
              radiusY: 0.5,
              force: 0.5,
            },
          ],
          activeTouches: [],
        },
      ]);
    });

    it('should work with three touches', async () => {
      const {page, server} = await getTestState();
      await page.goto(server.PREFIX + '/input/touchscreen.html');

      const touch1 = await page.touchscreen.touchStart(50, 50);
      await touch1.move(50, 100);
      await page.touchscreen.touchStart(20, 20);
      await touch1.end();
      const touch3 = await page.touchscreen.touchStart(20, 100);
      await touch3.move(60, 100);

      expect(
        await page.evaluate(async () => {
          // @ts-expect-error executed on the page
          return await globalThis.waitForEvents;
        }),
      ).toMatchObject([
        {
          type: 'pointerdown',
          x: 50,
          y: 50,
          width: 1,
          height: 1,
          altitudeAngle: 1.5707963267948966,
          azimuthAngle: 0,
          pressure: 0.5,
Repository: puppeteer/puppeteer
File: packages/puppeteer-core/src/bidi/Browser.ts (TypeScript)

Code before:
      width: clientWindowInfo.width,
      height: clientWindowInfo.height,
      windowState: clientWindowInfo.state,
    };
  }

  override async setWindowBounds(
    windowId: WindowId,
    windowBounds: WindowBounds,
  ): Promise<void> {
    let params: Bidi.Browser.SetClientWindowStateParameters | undefined;
    const windowState = windowBounds.windowState ?? 'normal';
    if (windowState === 'normal') {
      params = {
        clientWindow: windowId,
        state: 'normal',
        ...windowBounds,
      };
    } else {
      if (
        windowBounds.height !== undefined ||
        windowBounds.width !== undefined ||
        windowBounds.left !== undefined ||
        windowBounds.top !== undefined
      ) {
        throw new ProtocolError(
          'Window size and position are not supported for non-normal window states',
        );
      }
      params = {
        clientWindow: windowId,
        state: windowState,
      };
    }

    await this.#browserCore.setClientWindowState(params);
  }

  override targets(): Target[] {
    return [
      this.#target,
      ...this.browserContexts().flatMap(context => {
        return context.targets();
      }),
    ];
  }

  override target(): BidiBrowserTarget {
    return this.#target;
  }


Review comment (bug):
Do we need this explicit error? I would assume the underlying implementation to fail in this case or it just ignores this values?

Code after:
      width: clientWindowInfo.width,
      height: clientWindowInfo.height,
      windowState: clientWindowInfo.state,
    };
  }

  override async setWindowBounds(
    windowId: WindowId,
    windowBounds: WindowBounds,
  ): Promise<void> {
    let params: Bidi.Browser.SetClientWindowStateParameters | undefined;
    const windowState = windowBounds.windowState ?? 'normal';
    if (windowState === 'normal') {
      params = {
        clientWindow: windowId,
        state: 'normal',
        ...windowBounds,
      };
    } else {
      params = {
        clientWindow: windowId,
        state: windowState,
      };
    }

    await this.#browserCore.setClientWindowState(params);
  }

  override targets(): Target[] {
    return [
      this.#target,
      ...this.browserContexts().flatMap(context => {
        return context.targets();
      }),
    ];
  }

  override target(): BidiBrowserTarget {
    return this.#target;
  }

  override async disconnect(): Promise<void> {
    try {
      await this.#browserCore.session.end();
    } catch (error) {
      // Fail silently.
      debugError(error);
    } finally {
      this.connection.dispose();
    }
  }
Repository: puppeteer/puppeteer
File: packages/puppeteer-core/src/bidi/BrowserContext.ts (TypeScript)

Code before:
      options?.type === 'window'
        ? Bidi.BrowsingContext.CreateType.Window
        : Bidi.BrowsingContext.CreateType.Tab;

    const context = await this.userContext.createBrowsingContext(type, {
      background: options?.background,
    });
    const page = this.#pages.get(context)!;
    if (!page) {
      throw new Error('Page is not found');
    }
    if (this.#defaultViewport) {
      try {
        await page.setViewport(this.#defaultViewport);
      } catch {
        // No support for setViewport in Firefox.
      }
    }
    if (options?.type === 'window' && options?.windowBounds !== undefined) {
      try {
        await this.browser().setWindowBounds(
          context.windowId,
          options.windowBounds,
        );
      } catch {
        // Tolerate not supporting `browser.setClientWindowState`.
      }
    }

    return page;
  }

  override async close(): Promise<void> {
    assert(
      this.userContext.id !== UserContext.DEFAULT,
      'Default BrowserContext cannot be closed!',
    );

    try {
      await this.userContext.remove();
    } catch (error) {
      debugError(error);
    }

    this.#targets.clear();
  }

  override browser(): BidiBrowser {
    return this.#browser;
  }


Review comment (bug):
I think here and for similar places we should add a debug logging, just in case someone is testing somethings that consistency fails. 

Code after:
      options?.type === 'window'
        ? Bidi.BrowsingContext.CreateType.Window
        : Bidi.BrowsingContext.CreateType.Tab;

    const context = await this.userContext.createBrowsingContext(type, {
      background: options?.background,
    });
    const page = this.#pages.get(context)!;
    if (!page) {
      throw new Error('Page is not found');
    }
    if (this.#defaultViewport) {
      try {
        await page.setViewport(this.#defaultViewport);
      } catch (error) {
        // Tolerate not supporting `browsingContext.setViewport`. Only log it.
        debugError(error);
      }
    }
    if (options?.type === 'window' && options?.windowBounds !== undefined) {
      try {
        await this.browser().setWindowBounds(
          context.windowId,
          options.windowBounds,
        );
      } catch (error) {
        // Tolerate not supporting `browser.setClientWindowState`. Only log it.
        debugError(error);
      }
    }

    return page;
  }

  override async close(): Promise<void> {
    assert(
      this.userContext.id !== UserContext.DEFAULT,
      'Default BrowserContext cannot be closed!',
    );

    try {
      await this.userContext.remove();
    } catch (error) {
      debugError(error);
    }

    this.#targets.clear();
  }

  override browser(): BidiBrowser {
    return this.#browser;
Repository: puppeteer/puppeteer
File: packages/puppeteer-core/src/cdp/BrowserContext.ts (TypeScript)

Code before:
    return cookies.map(cookie => {
      return {
        ...cookie,
        partitionKey: cookie.partitionKey
          ? {
              sourceOrigin: cookie.partitionKey.topLevelSite,
              hasCrossSiteAncestor: cookie.partitionKey.hasCrossSiteAncestor,
            }
          : undefined,
        // TODO: remove sameParty as it is removed from Chrome.
        sameParty: cookie.sameParty ?? false,
      };
    });
  }

  override async setCookie(...cookies: CookieData[]): Promise<void> {
    return await this.#connection.send('Storage.setCookies', {
      browserContextId: this.#id,
      cookies: cookies.map(cookie => {
        const {sameSite, ...rest} = cookie;
        const sameSiteCdp = convertSameSiteFromPuppeteerToCdp(sameSite);
        const result: Protocol.Network.CookieParam = {
          ...rest,
          partitionKey: convertCookiesPartitionKeyFromPuppeteerToCdp(
            cookie.partitionKey,
          ),
        };
        if (sameSiteCdp) {
          result.sameSite = sameSiteCdp;
        }
        return result;
      }),
    });
  }

  public async setDownloadBehavior(
    downloadBehavior: DownloadBehavior,
  ): Promise<void> {
    await this.#connection.send('Browser.setDownloadBehavior', {
      behavior: downloadBehavior.policy,
      downloadPath: downloadBehavior.downloadPath,
      browserContextId: this.#id,
    });
  }
}


Review comment (suggestion):
We can do the following and JS handles is as expected, the last value is the correct one.
```suggestion
partitionKey: convertCookiesPartitionKeyFromPuppeteerToCdp(
cookie.partitionKey,
),
sameSite: convertSameSiteFromPuppeteerToCdp(cookie.sameSite)
```

Code after:
    return cookies.map(cookie => {
      return {
        ...cookie,
        partitionKey: cookie.partitionKey
          ? {
              sourceOrigin: cookie.partitionKey.topLevelSite,
              hasCrossSiteAncestor: cookie.partitionKey.hasCrossSiteAncestor,
            }
          : undefined,
        // TODO: remove sameParty as it is removed from Chrome.
        sameParty: cookie.sameParty ?? false,
      };
    });
  }

  override async setCookie(...cookies: CookieData[]): Promise<void> {
    return await this.#connection.send('Storage.setCookies', {
      browserContextId: this.#id,
      cookies: cookies.map(cookie => {
        return {
          ...cookie,
          partitionKey: convertCookiesPartitionKeyFromPuppeteerToCdp(
            cookie.partitionKey,
          ),
          sameSite: convertSameSiteFromPuppeteerToCdp(cookie.sameSite),
        };
      }),
    });
  }

  public async setDownloadBehavior(
    downloadBehavior: DownloadBehavior,
  ): Promise<void> {
    await this.#connection.send('Browser.setDownloadBehavior', {
      behavior: downloadBehavior.policy,
      downloadPath: downloadBehavior.downloadPath,
      browserContextId: this.#id,
    });
  }
}
Repository: puppeteer/puppeteer
File: packages/puppeteer-core/src/common/util.ts (TypeScript)

Code before:
  }
}

/**
 * @internal
 */
export async function getReadableFromProtocolStream(
  client: CDPSession,
  handle: string,
): Promise<ReadableStream<Uint8Array>> {
  return new ReadableStream({
    async pull(controller) {
      const {data, base64Encoded, eof} = await client.send('IO.read', {
        handle,
      });

      controller.enqueue(stringToTypedArray(data, base64Encoded ?? false));
      if (eof) {
        await client.send('IO.close', {handle});
        controller.close();
      }
    },
  });
}

const validDialogTypes = new Set([
  'alert',
  'confirm',
  'prompt',
  'beforeunload',
]);

/**
 * @internal
 */
export function validateDialogType(
  type: string,
): 'alert' | 'confirm' | 'prompt' | 'beforeunload' {
  let dialogType = null;

  if (validDialogTypes.has(type)) {
    dialogType = type;
  }
  assert(dialogType, `Unknown javascript dialog type: ${type}`);
  return dialogType as 'alert' | 'confirm' | 'prompt' | 'beforeunload';
}

/**
 * @internal
 */
export function timeout(ms: number, cause?: Error): Observable<never> {

Review comment (suggestion):
When outside the const need to have a name `VALIDATE_DIALOG_TYPES` 

Code after:
  }
}

/**
 * @internal
 */
export async function getReadableFromProtocolStream(
  client: CDPSession,
  handle: string,
): Promise<ReadableStream<Uint8Array>> {
  return new ReadableStream({
    async pull(controller) {
      const {data, base64Encoded, eof} = await client.send('IO.read', {
        handle,
      });

      controller.enqueue(stringToTypedArray(data, base64Encoded ?? false));
      if (eof) {
        await client.send('IO.close', {handle});
        controller.close();
      }
    },
  });
}

const VALID_DIALOG_TYPES = new Set([
  'alert',
  'confirm',
  'prompt',
  'beforeunload',
]);

/**
 * @internal
 */
export function validateDialogType(
  type: string,
): 'alert' | 'confirm' | 'prompt' | 'beforeunload' {
  let dialogType = null;

  if (VALID_DIALOG_TYPES.has(type)) {
    dialogType = type;
  }
  assert(dialogType, `Unknown javascript dialog type: ${type}`);
  return dialogType as 'alert' | 'confirm' | 'prompt' | 'beforeunload';
}

/**
 * @internal
 */
export function timeout(ms: number, cause?: Error): Observable<never> {
Repository: pydantic/pydantic-ai
File: tests/test_messages.py (Python)

Code before:
import sys

import pytest

from pydantic_ai.messages import (
    AudioUrl,
    BinaryContent,
    DocumentUrl,
    ImageUrl,
    ThinkingPartDelta,
    VideoUrl,
)


def test_image_url():
    image_url = ImageUrl(url='https://example.com/image.jpg')
    assert image_url.media_type == 'image/jpeg'
    assert image_url.format == 'jpeg'

    image_url = ImageUrl(url='https://example.com/image', _media_type='image/jpeg')
    assert image_url.media_type == 'image/jpeg'
    assert image_url.format == 'jpeg'


def test_video_url():
    video_url = VideoUrl(url='https://example.com/video.mp4')
    assert video_url.media_type == 'video/mp4'
    assert video_url.format == 'mp4'

    video_url = VideoUrl(url='https://example.com/video', _media_type='video/mp4')
    assert video_url.media_type == 'video/mp4'
    assert video_url.format == 'mp4'


@pytest.mark.parametrize(
    'url,is_youtube',
    [
        pytest.param('https://youtu.be/lCdaVNyHtjU', True, id='youtu.be'),
        pytest.param('https://www.youtube.com/lCdaVNyHtjU', True, id='www.youtube.com'),
        pytest.param('https://youtube.com/lCdaVNyHtjU', True, id='youtube.com'),
        pytest.param('https://dummy.com/video.mp4', False, id='dummy.com'),
    ],
)
def test_youtube_video_url(url: str, is_youtube: bool):
    video_url = VideoUrl(url=url)
    assert video_url.is_youtube is is_youtube

Review comment (suggestion):
This test was redundant with `test_video_url_invalid`
The same goes for `test_document_url` -> `test_document_url_invalid`

Code after:
import sys

import pytest

from pydantic_ai.messages import (
    AudioUrl,
    BinaryContent,
    DocumentUrl,
    ImageUrl,
    ThinkingPartDelta,
    VideoUrl,
)


def test_image_url():
    image_url = ImageUrl(url='https://example.com/image.jpg')
    assert image_url.media_type == 'image/jpeg'
    assert image_url.format == 'jpeg'

    image_url = ImageUrl(url='https://example.com/image', media_type='image/jpeg')
    assert image_url.media_type == 'image/jpeg'
    assert image_url.format == 'jpeg'


def test_video_url():
    video_url = VideoUrl(url='https://example.com/video.mp4')
    assert video_url.media_type == 'video/mp4'
    assert video_url.format == 'mp4'

    video_url = VideoUrl(url='https://example.com/video', media_type='video/mp4')
    assert video_url.media_type == 'video/mp4'
    assert video_url.format == 'mp4'


@pytest.mark.parametrize(
    'url,is_youtube',
    [
        pytest.param('https://youtu.be/lCdaVNyHtjU', True, id='youtu.be'),
        pytest.param('https://www.youtube.com/lCdaVNyHtjU', True, id='www.youtube.com'),
        pytest.param('https://youtube.com/lCdaVNyHtjU', True, id='youtube.com'),
        pytest.param('https://dummy.com/video.mp4', False, id='dummy.com'),
    ],
)
def test_youtube_video_url(url: str, is_youtube: bool):
    video_url = VideoUrl(url=url)
    assert video_url.is_youtube is is_youtube
Repository: pydantic/pydantic-ai
File: pydantic_ai_slim/pydantic_ai/providers/litellm.py (Python)

Code before:
from __future__ import annotations as _annotations

from typing import overload

from httpx import AsyncClient as AsyncHTTPClient
from openai import AsyncOpenAI

from pydantic_ai.profiles import ModelProfile
from pydantic_ai.profiles.openai import OpenAIJsonSchemaTransformer, OpenAIModelProfile, openai_model_profile
from pydantic_ai.providers import Provider
from pydantic_ai_slim.pydantic_ai.models import cached_async_http_client

try:
    from openai import AsyncOpenAI
except ImportError as _import_error:  # pragma: no cover
    raise ImportError(
        'Please install the `openai` package to use the LiteLLM provider, '
        'you can use the `openai` optional group — `pip install "pydantic-ai-slim[openai]"`'
    ) from _import_error


class LiteLLMProvider(Provider[AsyncOpenAI]):
    """Provider for LiteLLM API."""

    @property
    def name(self) -> str:
        return 'litellm'

    @property
    def base_url(self) -> str:
        return self._base_url

    @property
    def client(self) -> AsyncOpenAI:
        return self._client


Review comment (performance):
```suggestion
from pydantic_ai.models import cached_async_http_client
```

Code after:
from __future__ import annotations as _annotations

from typing import overload

from httpx import AsyncClient as AsyncHTTPClient
from openai import AsyncOpenAI

from pydantic_ai.models import cached_async_http_client
from pydantic_ai.profiles import ModelProfile
from pydantic_ai.profiles.amazon import amazon_model_profile
from pydantic_ai.profiles.anthropic import anthropic_model_profile
from pydantic_ai.profiles.cohere import cohere_model_profile
from pydantic_ai.profiles.deepseek import deepseek_model_profile
from pydantic_ai.profiles.google import google_model_profile
from pydantic_ai.profiles.grok import grok_model_profile
from pydantic_ai.profiles.groq import groq_model_profile
from pydantic_ai.profiles.meta import meta_model_profile
from pydantic_ai.profiles.mistral import mistral_model_profile
from pydantic_ai.profiles.moonshotai import moonshotai_model_profile
from pydantic_ai.profiles.openai import OpenAIJsonSchemaTransformer, OpenAIModelProfile, openai_model_profile
from pydantic_ai.profiles.qwen import qwen_model_profile
from pydantic_ai.providers import Provider

try:
    from openai import AsyncOpenAI
except ImportError as _import_error:  # pragma: no cover
    raise ImportError(
        'Please install the `openai` package to use the LiteLLM provider, '
        'you can use the `openai` optional group — `pip install "pydantic-ai-slim[openai]"`'
    ) from _import_error


class LiteLLMProvider(Provider[AsyncOpenAI]):
    """Provider for LiteLLM API."""

    @property