ronantakizawa/codereview-bench huggingface dataset:
Repository: tiangolo/fastapi
File: tests/test_response_model_default_factory.py (Python)

Code before:
    "/response_model_has_default_factory_return_model",
    response_model=ResponseModel,
)
async def response_model_has_default_factory_return_model():
    return ResponseModel()


client = TestClient(app)


def test_response_model_has_default_factory_return_dict():
    response = client.get("/response_model_has_default_factory_return_dict")

    assert response.status_code == 200, response.text

    assert response.json()["code"] == 200
    assert response.json()["message"] in messages


def test_response_model_has_default_factory_return_model():
    response = client.get("/response_model_has_default_factory_return_model")

    assert response.status_code == 200, response.text

    assert response.json()["code"] == 200
    assert response.json()["message"] in messages


Review comment (suggestion):
```suggestion
    assert response.json()["message"] == "Successful operation."
```

Code after:
from fastapi import FastAPI
from fastapi.testclient import TestClient
from pydantic import BaseModel, Field

app = FastAPI()


class ResponseModel(BaseModel):
    code: int = 200
    message: str = Field(default_factory=lambda: "Successful operation.")


@app.get(
    "/response_model_has_default_factory_return_dict",
    response_model=ResponseModel,
)
async def response_model_has_default_factory_return_dict():
    return {"code": 200}


@app.get(
    "/response_model_has_default_factory_return_model",
    response_model=ResponseModel,
)
async def response_model_has_default_factory_return_model():
    return ResponseModel()


client = TestClient(app)


def test_response_model_has_default_factory_return_dict():
    response = client.get("/response_model_has_default_factory_return_dict")

    assert response.status_code == 200, response.text

    assert response.json()["code"] == 200
    assert response.json()["message"] == "Successful operation."


def test_response_model_has_default_factory_return_model():
    response = client.get("/response_model_has_default_factory_return_model")

    assert response.status_code == 200, response.text

    assert response.json()["code"] == 200
    assert response.json()["message"] == "Successful operation."
Repository: tiangolo/fastapi
File: docs_src/app_testing/tutorial003.py (Python)

Code before:
from contextlib import asynccontextmanager

from fastapi import FastAPI
from fastapi.testclient import TestClient

items = {}


@asynccontextmanager
async def lifespan(app: FastAPI):
    items["foo"] = {"name": "Fighters"}
    items["bar"] = {"name": "Tenders"}
    yield
    # clean up items or other work
    ...


app = FastAPI(lifespan=lifespan)

# startup event and shutdown event are deprecated, you should use lifespan instead
# @app.on_event("startup")
# async def startup_event():
#     items["foo"] = {"name": "Fighters"}
#     items["bar"] = {"name": "Tenders"}


@app.get("/items/{item_id}")
async def read_items(item_id: str):
    return items[item_id]


def test_read_items():
    with TestClient(app) as client:
        response = client.get("/items/foo")
        assert response.status_code == 200
        assert response.json() == {"name": "Fighters"}


Review comment (refactor):
To ensure the code with startup\shutdown is still tested, I would split this code example into two separate: first is for `lifespan`, second is for `startup` and `shutdown`.
We can add the warning that it's deprecated outside the code block.
What do you think?

Code after:
from fastapi import FastAPI
from fastapi.testclient import TestClient

app = FastAPI()

items = {}


@app.on_event("startup")
async def startup_event():
    items["foo"] = {"name": "Fighters"}
    items["bar"] = {"name": "Tenders"}


@app.get("/items/{item_id}")
async def read_items(item_id: str):
    return items[item_id]


def test_read_items():
    with TestClient(app) as client:
        response = client.get("/items/foo")
        assert response.status_code == 200
        assert response.json() == {"name": "Fighters"}
Repository: tiangolo/fastapi
File: tests/test_tutorial/test_settings/test_tutorial001.py (Python)

Code before:
import importlib

import pytest
from fastapi.testclient import TestClient
from pytest import MonkeyPatch

from ...utils import needs_pydanticv1, needs_pydanticv2


@pytest.fixture(
    params=[
        pytest.param("tutorial001", marks=needs_pydanticv2),
        pytest.param("tutorial001_pv1", marks=needs_pydanticv1),
    ],
)
def get_app(request: pytest.FixtureRequest):
    def app_wrapper():
        mod = importlib.import_module(f"docs_src.settings.{request.param}")
        return mod.app

    return app_wrapper


def test_settings(get_app, monkeypatch: MonkeyPatch):
    monkeypatch.setenv("ADMIN_EMAIL", "admin@example.com")

    app = get_app()
    client = TestClient(app)
    response = client.get("/info")
    assert response.status_code == 200, response.text
    assert response.json() == {
        "app_name": "Awesome API",
        "admin_email": "admin@example.com",
        "items_per_user": 50,
    }


Review comment (suggestion):
I think that it is better to monkeypatch `get_app()`, instead of test_settings:

```python

@pytest.fixture(
    name="app",
    params=[
        pytest.param("tutorial001", marks=needs_pydanticv2),
        pytest.param("tutorial001_pv1", marks=needs_pydanticv1),
    ],
)
def get_app(request: pytest.FixtureRequest, monkeypatch: MonkeyPatch):
    monkeypatch.setenv("ADMIN_EMAIL", "admin@example.com")
    mod = importlib.import_module(f"docs_src.settings.{request.param}")
    return mod.app


def test_settings(app):
    client = TestClient(app)
    response = client.get("/info")
    assert response.status_code == 200, response.text
    assert response.json() == {
        "app_name": "Awesome API",
        "admin_email": "admin@example.com",
        "items_per_user": 50,
    }

```

Code after:
import importlib

import pytest
from fastapi.testclient import TestClient
from pytest import MonkeyPatch

from ...utils import needs_pydanticv1, needs_pydanticv2


@pytest.fixture(
    name="app",
    params=[
        pytest.param("tutorial001", marks=needs_pydanticv2),
        pytest.param("tutorial001_pv1", marks=needs_pydanticv1),
    ],
)
def get_app(request: pytest.FixtureRequest, monkeypatch: MonkeyPatch):
    monkeypatch.setenv("ADMIN_EMAIL", "admin@example.com")
    mod = importlib.import_module(f"docs_src.settings.{request.param}")
    return mod.app


def test_settings(app):
    client = TestClient(app)
    response = client.get("/info")
    assert response.status_code == 200, response.text
    assert response.json() == {
        "app_name": "Awesome API",
        "admin_email": "admin@example.com",
        "items_per_user": 50,
    }
Repository: tiangolo/fastapi
File: tests/test_form_default.py (Python)

Code before:
from typing import Optional

from fastapi import FastAPI, File, Form
from starlette.testclient import TestClient
from typing_extensions import Annotated

app = FastAPI()
client = TestClient(app)


@app.post("/urlencoded")
async def post_url_encoded(age: Annotated[Optional[int], Form()] = None):
    return age


def test_form_default_url_encoded():
    response = client.post("/urlencoded", data={"age": ""})
    assert response.status_code == 200
    assert response.text == "null"


@app.post("/multipart")
async def post_multi_part(
    age: Annotated[Optional[int], Form()] = None,
    file: Annotated[Optional[bytes], File()] = None,
):
    assert file is None
    assert age is None


def test_form_default_multi_part():
    response = client.post("/multipart", data={"age": ""})
    assert response.status_code == 200


@app.post("/multipart-file-first")
async def post_multi_part_file_first(
    file: Annotated[Optional[bytes], File()] = None,
    age: Annotated[Optional[int], Form()] = None,
):
    assert file is None
    assert age is None



Review comment (suggestion):
This looks odd to me.
If you pass a string to an integer field, I would expect a `4xx` error code.

Code after:
from typing import Optional

from fastapi import FastAPI, File, Form
from starlette.testclient import TestClient
from typing_extensions import Annotated

app = FastAPI()


@app.post("/urlencoded")
async def post_url_encoded(age: Annotated[Optional[int], Form()] = None):
    return age


@app.post("/multipart")
async def post_multi_part(
    age: Annotated[Optional[int], Form()] = None,
    file: Annotated[Optional[bytes], File()] = None,
):
    return {"file": file, "age": age}


client = TestClient(app)


def test_form_default_url_encoded():
    response = client.post("/urlencoded", data={"age": ""})
    assert response.status_code == 200
    assert response.text == "null"


def test_form_default_multi_part():
    response = client.post("/multipart", data={"age": ""})
    assert response.status_code == 200
    assert response.json() == {"file": None, "age": None}
Repository: tiangolo/fastapi
File: tests/test_openapi_schema_type.py (Python)

Code before:
from typing import List, Optional, Union

import pytest
from fastapi.openapi.constants import TypeValue
from fastapi.openapi.models import Schema


@pytest.mark.parametrize(
    "type_value",
    [
        "array",
        ["string", "null"],
        None,
    ],
)
def test_allowed_schema_type(
    type_value: Optional[Union[TypeValue, List[TypeValue]]],
) -> None:
    """Test that Schema accepts TypeValue, List[TypeValue] and None for type field."""
    schema = Schema(type=type_value)
    assert schema.type == type_value


def test_invlid_type_value() -> None:
    """Test that Schema raises ValueError for invalid type values."""
    with pytest.raises(ValueError, match="2 validation errors for Schema"):
        Schema(type=True)  # type: ignore[arg-type]


Review comment (suggestion):
```suggestion
def test_invalid_type_value() -> None:
```

Code after:
from typing import List, Optional, Union

import pytest
from fastapi.openapi.models import Schema, SchemaType


@pytest.mark.parametrize(
    "type_value",
    [
        "array",
        ["string", "null"],
        None,
    ],
)
def test_allowed_schema_type(
    type_value: Optional[Union[SchemaType, List[SchemaType]]],
) -> None:
    """Test that Schema accepts SchemaType, List[SchemaType] and None for type field."""
    schema = Schema(type=type_value)
    assert schema.type == type_value


def test_invalid_type_value() -> None:
    """Test that Schema raises ValueError for invalid type values."""
    with pytest.raises(ValueError, match="2 validation errors for Schema"):
        Schema(type=True)  # type: ignore[arg-type]
Repository: tiangolo/fastapi
File: fastapi/dependencies/utils.py (Python)

Code before:
    elif any(isinstance(f.field_info, params.Form) for f in flat_dependant.body_params):
        BodyFieldInfo = params.Form
    else:
        BodyFieldInfo = params.Body

        body_param_media_types = [
            f.field_info.media_type
            for f in flat_dependant.body_params
            if isinstance(f.field_info, params.Body)
        ]
        if len(set(body_param_media_types)) == 1:
            BodyFieldInfo_kwargs["media_type"] = body_param_media_types[0]
    final_field = create_model_field(
        name="body",
        type_=BodyModel,
        required=required,
        alias="body",
        field_info=BodyFieldInfo(**BodyFieldInfo_kwargs),
    )
    return final_field


def _is_typealiastype(tp: Any, /) -> TypeGuard[TypeAliasType]:
    in_typing = hasattr(typing, "TypeAliasType")
    in_typing_extensions = hasattr(typing_extensions, "TypeAliasType")
    is_typealiastype = False
    if in_typing and in_typing_extensions:
        if getattr(typing, "TypeAliasType", None) is getattr(
            typing_extensions, "TypeAliasType", None
        ):  # pragma: no cover
            is_typealiastype = isinstance(tp, typing.TypeAliasType)  # type: ignore [attr-defined]
        else:
            is_typealiastype = isinstance(
                tp,
                (typing.TypeAliasType, typing_extensions.TypeAliasType),  # type: ignore [attr-defined]
            )
    elif in_typing and not in_typing_extensions:  # pragma: no cover
        is_typealiastype = isinstance(tp, typing.TypeAliasType)  # type: ignore [attr-defined]
    elif not in_typing and in_typing_extensions:
        is_typealiastype = isinstance(tp, typing_extensions.TypeAliasType)
    if sys.version_info[:2] == (3, 10):
        return type(tp) is not GenericAlias and is_typealiastype
    return is_typealiastype


Review comment (suggestion):
```suggestion

    is_typealiastype = False  # Default: assume not a TypeAliasType
```

Code after:
    if not required:
        BodyFieldInfo_kwargs["default"] = None
    if any(isinstance(f.field_info, params.File) for f in flat_dependant.body_params):
        BodyFieldInfo: type[params.Body] = params.File
    elif any(isinstance(f.field_info, params.Form) for f in flat_dependant.body_params):
        BodyFieldInfo = params.Form
    else:
        BodyFieldInfo = params.Body

        body_param_media_types = [
            f.field_info.media_type
            for f in flat_dependant.body_params
            if isinstance(f.field_info, params.Body)
        ]
        if len(set(body_param_media_types)) == 1:
            BodyFieldInfo_kwargs["media_type"] = body_param_media_types[0]
    final_field = create_model_field(
        name="body",
        type_=BodyModel,
        required=required,
        alias="body",
        field_info=BodyFieldInfo(**BodyFieldInfo_kwargs),
    )
    return final_field


def get_validation_alias(field: ModelField) -> str:
    va = getattr(field, "validation_alias", None)
    return va or field.alias
Repository: tiangolo/fastapi
File: fastapi/dependencies/utils.py (Python)

Code before:
    elif lenient_issubclass(type_annotation, SecurityScopes):
        dependant.security_scopes_param_name = param_name
        return True
    return None


@dataclass
class ParamDetails:
    type_annotation: Any
    depends: Optional[params.Depends]
    field: Optional[ModelField]


def analyze_param(
    *,
    param_name: str,
    annotation: Any,
    value: Any,
    is_path_param: bool,
) -> ParamDetails:
    field_info = None
    depends = None
    type_annotation: Any = Any
    use_annotation: Any = Any
    if _is_typealiastype(annotation):
        # unpack in case py3.12 type syntax is used
        annotation = annotation.__value__
    if annotation is not inspect.Signature.empty:
        use_annotation = annotation
        type_annotation = annotation
    # Extract Annotated info
    if get_origin(use_annotation) is Annotated:
        annotated_args = get_args(annotation)
        type_annotation = annotated_args[0]
        fastapi_annotations = [
            arg
            for arg in annotated_args[1:]
            if isinstance(arg, (FieldInfo, params.Depends))
        ]
        fastapi_specific_annotations = [
            arg
            for arg in fastapi_annotations
            if isinstance(arg, (params.Param, params.Body, params.Depends))
        ]
        if fastapi_specific_annotations:
            fastapi_annotation: Union[FieldInfo, params.Depends, None] = (
                fastapi_specific_annotations[-1]
            )
        else:
            fastapi_annotation = None
        # Set default for Annotated FieldInfo

Review comment (suggestion):
```suggestion
        # unpack in case PEP 695 type syntax is used
```

Code after:
    elif lenient_issubclass(type_annotation, SecurityScopes):
        dependant.security_scopes_param_name = param_name
        return True
    return None


@dataclass
class ParamDetails:
    type_annotation: Any
    depends: Optional[params.Depends]
    field: Optional[ModelField]


def analyze_param(
    *,
    param_name: str,
    annotation: Any,
    value: Any,
    is_path_param: bool,
) -> ParamDetails:
    field_info = None
    depends = None
    type_annotation: Any = Any
    use_annotation: Any = Any
    if is_typealiastype(annotation):
        # unpack in case PEP 695 type syntax is used
        annotation = annotation.__value__
    if annotation is not inspect.Signature.empty:
        use_annotation = annotation
        type_annotation = annotation
    # Extract Annotated info
    if get_origin(use_annotation) is Annotated:
        annotated_args = get_args(annotation)
        type_annotation = annotated_args[0]
        fastapi_annotations = [
            arg
            for arg in annotated_args[1:]
            if isinstance(arg, (FieldInfo, params.Depends))
        ]
        fastapi_specific_annotations = [
            arg
            for arg in fastapi_annotations
            if isinstance(
                arg,
                (
                    params.Param,
                    params.Body,
                    params.Depends,
                ),
            )
        ]
Repository: tiangolo/fastapi
File: scripts/docs.py (Python)

Code before:
from html.parser import HTMLParser
from http.server import HTTPServer, SimpleHTTPRequestHandler
from importlib import metadata
from multiprocessing import Pool
from pathlib import Path
from typing import Any, Dict, List, Optional, Union

import mkdocs.utils
import typer
import yaml
from jinja2 import Template
from ruff.__main__ import find_ruff_bin
from slugify import slugify as py_slugify

logging.basicConfig(level=logging.INFO)

app = typer.Typer()

mkdocs_name = "mkdocs.yml"

missing_translation_snippet = """
{!../../docs/missing-translation.md!}
"""

non_translated_sections = (
    "reference/",
    "release-notes.md",
    "fastapi-people.md",
    "external-links.md",
    "newsletter.md",
    "management-tasks.md",
    "management.md",
    "contributing.md",
)

docs_path = Path("docs")
en_docs_path = Path("docs/en")
en_config_path: Path = en_docs_path / mkdocs_name
site_path = Path("site").absolute()
build_site_path = Path("site_build").absolute()

header_pattern = re.compile(r"^(#{1,6}) (.+?)(?:\s*\{\s*(#.*)\s*\})?\s*$")
header_with_permalink_pattern = re.compile(r"^(#{1,6}) (.+?)(\s*\{\s*#.*\s*\})\s*$")
code_block3_pattern = re.compile(r"^\s*```")
code_block4_pattern = re.compile(r"^\s*````")


class VisibleTextExtractor(HTMLParser):
    """Extract visible text from a string with HTML tags."""

    def __init__(self):

Review comment (suggestion):
To be OS agnostic:
```suggestion
    "reference",
```
assuming we have no "normal" pages starting with "reference" (I don't think that's a real issue)

Code after:
from http.server import HTTPServer, SimpleHTTPRequestHandler
from multiprocessing import Pool
from pathlib import Path
from typing import Any, Dict, List, Optional, Union

import mkdocs.utils
import typer
import yaml
from jinja2 import Template
from ruff.__main__ import find_ruff_bin
from slugify import slugify as py_slugify

logging.basicConfig(level=logging.INFO)

app = typer.Typer()

mkdocs_name = "mkdocs.yml"

missing_translation_snippet = """
{!../../docs/missing-translation.md!}
"""

non_translated_sections = (
    f"reference{os.sep}",
    "release-notes.md",
    "fastapi-people.md",
    "external-links.md",
    "newsletter.md",
    "management-tasks.md",
    "management.md",
    "contributing.md",
)

docs_path = Path("docs")
en_docs_path = Path("docs/en")
en_config_path: Path = en_docs_path / mkdocs_name
site_path = Path("site").absolute()
build_site_path = Path("site_build").absolute()

header_pattern = re.compile(r"^(#{1,6}) (.+?)(?:\s*\{\s*(#.*)\s*\})?\s*$")
header_with_permalink_pattern = re.compile(r"^(#{1,6}) (.+?)(\s*\{\s*#.*\s*\})\s*$")
code_block3_pattern = re.compile(r"^\s*```")
code_block4_pattern = re.compile(r"^\s*````")


class VisibleTextExtractor(HTMLParser):
    """Extract visible text from a string with HTML tags."""

    def __init__(self):
        super().__init__()
        self.text_parts = []
Repository: tiangolo/fastapi
File: docs_src/handling_errors/tutorial005.py (Python)

Code before:
from fastapi import FastAPI, Request
from fastapi.encoders import jsonable_encoder
from fastapi.exceptions import RequestValidationError
from fastapi.responses import JSONResponse
from pydantic import BaseModel

app = FastAPI()


@app.exception_handler(RequestValidationError)
async def validation_exception_handler(request: Request, exc: RequestValidationError):
    return JSONResponse(
        # 422 = fastapi.status.HTTP_422_UNPROCESSABLE_CONTENT (RFC 9110,
        # Starlette >=0.48), previously HTTP_422_UNPROCESSABLE_ENTITY
        status_code=422,
        content=jsonable_encoder({"detail": exc.errors(), "body": exc.body}),
    )


class Item(BaseModel):
    title: str
    size: int


@app.post("/items/")
async def create_item(item: Item):
    return item


Review comment (bug):
We have to be carefull here, this script is referenced in the docs [here](https://fastapi.tiangolo.com/tutorial/handling-errors/#use-the-requestvalidationerror-body), and by adding more lines, the wrong line will be highlighted in the docs (used to be L14 which is now L16).

Anyway, I don't think we need this comment here, or elsewhere where we're switching to 422. Let's just edit to `status_code=422` without the comments.

Code after:
from fastapi import FastAPI, Request
from fastapi.encoders import jsonable_encoder
from fastapi.exceptions import RequestValidationError
from fastapi.responses import JSONResponse
from pydantic import BaseModel

app = FastAPI()


@app.exception_handler(RequestValidationError)
async def validation_exception_handler(request: Request, exc: RequestValidationError):
    return JSONResponse(
        status_code=422,
        content=jsonable_encoder({"detail": exc.errors(), "body": exc.body}),
    )


class Item(BaseModel):
    title: str
    size: int


@app.post("/items/")
async def create_item(item: Item):
    return item
Repository: tiangolo/fastapi
File: tests/test_router_circular_import.py (Python)

Code before:
import pytest
from fastapi import APIRouter, FastAPI


def test_router_circular_import():
    app = FastAPI()
    router = APIRouter()

    app.include_router(router)
    with pytest.raises(AssertionError, match="Cannot include router into itself"):
        router.include_router(router)


Review comment (bug):
```suggestion
import pytest
from fastapi import APIRouter


def test_router_circular_import():
    router = APIRouter()

    with pytest.raises(AssertionError, match="Cannot include router into itself"):
        router.include_router(router)
```

`app` is not needed here, and calling `router.include_router` after `app.include_router(router)` wrong

Code after:
import pytest
from fastapi import APIRouter


def test_router_circular_import():
    router = APIRouter()

    with pytest.raises(
        AssertionError,
        match="Cannot include the same APIRouter instance into itself. Did you mean to include a different router?",
    ):
        router.include_router(router)
Repository: tiangolo/fastapi
File: docs_src/custom_response/tutorial007_py310.py (Python)

Code before:
import asyncio

from fastapi import FastAPI
from fastapi.responses import StreamingResponse

app = FastAPI()


async def fake_video_streamer():
    for i in range(10):
        yield b"some fake video bytes"
        await asyncio.sleep(0)  # simulate async work


@app.get("/")
async def main():
    return StreamingResponse(fake_video_streamer())


Review comment (bug):
```suggestion
        await asyncio.sleep(0)
```

The comment is incorrect - this is not for simulation. Let's not add comment here, it's explained in the note below the code example

Code after:
import anyio
from fastapi import FastAPI
from fastapi.responses import StreamingResponse

app = FastAPI()


async def fake_video_streamer():
    for i in range(10):
        yield b"some fake video bytes"
        await anyio.sleep(0)


@app.get("/")
async def main():
    return StreamingResponse(fake_video_streamer())