ronantakizawa/codereview-bench huggingface dataset:
Repository: Shpota/goxygen
File: templates/server/web/app_test.go (Go)

Code before:
	},
	{
		Name:    "Tech2",
		Details: "Details2",
	},
}

type MockDb struct {
	called     int
	returnTech []*model.Technology
	returnErr  error
}

func (m *MockDb) GetTechnologies() ([]*model.Technology, error) {
	m.called++
	return m.returnTech, m.returnErr
}

func TestApp_GetTechnologies(t *testing.T) {
	mockDb := &MockDb{
		returnTech: expectedTechnologies,
	}
	app := App{d: mockDb}

	req := buildTechnologiesRequest(t)
	recorder := httptest.NewRecorder()

	app.GetTechnologies(recorder, req)

	verifyDbWasCalledOnce(t, mockDb)
	status := recorder.Code
	if status != http.StatusOK {
		t.Errorf("handler returned wrong status code: got %v want %v", status, http.StatusOK)
	}

	expected := `[{"name":"Tech1","details":"Details1"},{"name":"Tech2","details":"Details2"}]` + "\n"
	body := recorder.Body.String()
	if body != expected {
		t.Errorf("handler returned unexpected body: got %v want %v", body, expected)
	}
}

func TestApp_GetTechnologies_WithDBError(t *testing.T) {
	mockDb := &MockDb{
		returnTech: nil,
		returnErr:  errors.New("unknown error"),
	}
	app := App{d: mockDb}

	req := buildTechnologiesRequest(t)
	recorder := httptest.NewRecorder()

Review comment (suggestion):
I would suggest to use a shorter variable name here and generally in the Go code. Although it is unusual to see single-letter-names coming from a different programming language, Go encourages this approach and I try to follow it. You can read more on why it is preferred in [this presentation](https://talks.golang.org/2014/names.slide#6) from Google.

My suggestion would be:
```go
w := httptest.NewRecorder()
```
"w" because `GetTechnologies` consumes an instance of "writer" and the recorder here is another representation of writer.

Code after:

func (m *MockDb) GetTechnologies() ([]*model.Technology, error) {
	return m.tech, m.err
}

func TestApp_GetTechnologies(t *testing.T) {
	app := App{d: &MockDb{
		tech: []*model.Technology{
			{"Tech1", "Details1"},
			{"Tech2", "Details2"},
		},
	}}

	r, _ := http.NewRequest("GET", "/api/technologies", nil)
	w := httptest.NewRecorder()

	app.GetTechnologies(w, r)

	if w.Code != http.StatusOK {
		t.Errorf("handler returned wrong status code: got %v want %v", w.Code, http.StatusOK)
	}

	want := `[{"name":"Tech1","details":"Details1"},{"name":"Tech2","details":"Details2"}]` + "\n"
	if got := w.Body.String(); got != want {
		t.Errorf("handler returned unexpected body: got %v want %v", got, want)
	}
}

func TestApp_GetTechnologies_WithDBError(t *testing.T) {
	app := App{d: &MockDb{
		tech: nil,
		err:  errors.New("unknown error"),
	}}

	r, _ := http.NewRequest("GET", "/api/technologies", nil)
	w := httptest.NewRecorder()

	app.GetTechnologies(w, r)

	if w.Code != http.StatusInternalServerError {
		t.Errorf("handler returned wrong status code: got %v want %v", w.Code, http.StatusOK)
	}

}
Repository: Shpota/goxygen
File: templates/server/web/app_test.go (Go)

Code before:

func (m *MockDb) GetTechnologies() ([]*model.Technology, error) {
	m.called++
	return m.returnTech, m.returnErr
}

func TestApp_GetTechnologies(t *testing.T) {
	mockDb := &MockDb{
		returnTech: expectedTechnologies,
	}
	app := App{d: mockDb}

	req := buildTechnologiesRequest(t)
	recorder := httptest.NewRecorder()

	app.GetTechnologies(recorder, req)

	verifyDbWasCalledOnce(t, mockDb)
	status := recorder.Code
	if status != http.StatusOK {
		t.Errorf("handler returned wrong status code: got %v want %v", status, http.StatusOK)
	}

	expected := `[{"name":"Tech1","details":"Details1"},{"name":"Tech2","details":"Details2"}]` + "\n"
	body := recorder.Body.String()
	if body != expected {
		t.Errorf("handler returned unexpected body: got %v want %v", body, expected)
	}
}

func TestApp_GetTechnologies_WithDBError(t *testing.T) {
	mockDb := &MockDb{
		returnTech: nil,
		returnErr:  errors.New("unknown error"),
	}
	app := App{d: mockDb}

	req := buildTechnologiesRequest(t)
	recorder := httptest.NewRecorder()

	app.GetTechnologies(recorder, req)

	verifyDbWasCalledOnce(t, mockDb)
	status := recorder.Code
	if status != http.StatusInternalServerError {
		t.Errorf("handler returned wrong status code: got %v want %v", status, http.StatusOK)
	}

}

func buildTechnologiesRequest(t *testing.T) *http.Request {

Review comment (suggestion):
I think it would be more readable following the "got/want" naming (like it is used in the error message).

```go
if got != want { ... }
```
You can also include the `recorder.Body.String()` to the if statement as it is only needed in the `if` block:
```go
if got := w.Body.String(); got != want { ... }
```

Code after:

	r, _ := http.NewRequest("GET", "/api/technologies", nil)
	w := httptest.NewRecorder()

	app.GetTechnologies(w, r)

	if w.Code != http.StatusOK {
		t.Errorf("handler returned wrong status code: got %v want %v", w.Code, http.StatusOK)
	}

	want := `[{"name":"Tech1","details":"Details1"},{"name":"Tech2","details":"Details2"}]` + "\n"
	if got := w.Body.String(); got != want {
		t.Errorf("handler returned unexpected body: got %v want %v", got, want)
	}
}

func TestApp_GetTechnologies_WithDBError(t *testing.T) {
	app := App{d: &MockDb{
		tech: nil,
		err:  errors.New("unknown error"),
	}}

	r, _ := http.NewRequest("GET", "/api/technologies", nil)
	w := httptest.NewRecorder()

	app.GetTechnologies(w, r)

	if w.Code != http.StatusInternalServerError {
		t.Errorf("handler returned wrong status code: got %v want %v", w.Code, http.StatusOK)
	}

}
Repository: Shpota/goxygen
File: templates/server/web/app_test.go (Go)

Code before:
package web

import (
	"errors"
	"net/http"
	"net/http/httptest"
	"project-name/model"
	"testing"
)

var expectedTechnologies = []*model.Technology{
	{
		Name:    "Tech1",
		Details: "Details1",
	},
	{
		Name:    "Tech2",
		Details: "Details2",
	},
}

type MockDb struct {
	called     int
	returnTech []*model.Technology
	returnErr  error
}

func (m *MockDb) GetTechnologies() ([]*model.Technology, error) {
	m.called++
	return m.returnTech, m.returnErr
}

func TestApp_GetTechnologies(t *testing.T) {
	mockDb := &MockDb{
		returnTech: expectedTechnologies,
	}
	app := App{d: mockDb}

Review comment (bug):
I think checking how many times the method was called is not required. First of all, it is only a template and the user might not want to check this. Second, it is a debatable point if this check should be applied or not. Say, if it is applied and we expect someone implementing this function wrongly by calling the db several times we assume that we have no access to the code and we apply "black box" testing. Which is obviously not the case because it is a unit test. Does that sound fine to you?

Code after:
package web

import (
	"errors"
	"net/http"
	"net/http/httptest"
	"project-name/model"
	"testing"
)

type MockDb struct {
	tech []*model.Technology
	err  error
}

func (m *MockDb) GetTechnologies() ([]*model.Technology, error) {
	return m.tech, m.err
}

func TestApp_GetTechnologies(t *testing.T) {
	app := App{d: &MockDb{
		tech: []*model.Technology{
			{"Tech1", "Details1"},
			{"Tech2", "Details2"},
		},
	}}

	r, _ := http.NewRequest("GET", "/api/technologies", nil)
	w := httptest.NewRecorder()

	app.GetTechnologies(w, r)

	if w.Code != http.StatusOK {
		t.Errorf("handler returned wrong status code: got %v want %v", w.Code, http.StatusOK)
	}

	want := `[{"name":"Tech1","details":"Details1"},{"name":"Tech2","details":"Details2"}]` + "\n"
Repository: Shpota/goxygen
File: templates/server/web/app_test.go (Go)

Code before:
	status := recorder.Code
	if status != http.StatusOK {
		t.Errorf("handler returned wrong status code: got %v want %v", status, http.StatusOK)
	}

	expected := `[{"name":"Tech1","details":"Details1"},{"name":"Tech2","details":"Details2"}]` + "\n"
	body := recorder.Body.String()
	if body != expected {
		t.Errorf("handler returned unexpected body: got %v want %v", body, expected)
	}
}

func TestApp_GetTechnologies_WithDBError(t *testing.T) {
	mockDb := &MockDb{
		returnTech: nil,
		returnErr:  errors.New("unknown error"),
	}
	app := App{d: mockDb}

	req := buildTechnologiesRequest(t)
	recorder := httptest.NewRecorder()

	app.GetTechnologies(recorder, req)

	verifyDbWasCalledOnce(t, mockDb)
	status := recorder.Code
	if status != http.StatusInternalServerError {
		t.Errorf("handler returned wrong status code: got %v want %v", status, http.StatusOK)
	}

}

func buildTechnologiesRequest(t *testing.T) *http.Request {
	req, err := http.NewRequest("GET", "/api/technologies", nil)
	if err != nil {
		t.Fatal(err)
	}
	return req
}

func verifyDbWasCalledOnce(t *testing.T, db *MockDb) {
	if db.called != 1 {
		t.Errorf("db was called %d times instead of once", db.called)
	}
}


Review comment (refactor):
II think you can omit extracting Code to a variable. If you rename `recorder` to `w` (see my other comment) it will be short enough to use directly.

```go
if w.Code != http.StatusOK { ... }
```

Code after:
package web

import (
	"errors"
	"net/http"
	"net/http/httptest"
	"project-name/model"
	"testing"
)

type MockDb struct {
	tech []*model.Technology
	err  error
}

func (m *MockDb) GetTechnologies() ([]*model.Technology, error) {
	return m.tech, m.err
}

func TestApp_GetTechnologies(t *testing.T) {
	app := App{d: &MockDb{
		tech: []*model.Technology{
			{"Tech1", "Details1"},
			{"Tech2", "Details2"},
		},
	}}

	r, _ := http.NewRequest("GET", "/api/technologies", nil)
	w := httptest.NewRecorder()

	app.GetTechnologies(w, r)

	if w.Code != http.StatusOK {
		t.Errorf("handler returned wrong status code: got %v want %v", w.Code, http.StatusOK)
	}

	want := `[{"name":"Tech1","details":"Details1"},{"name":"Tech2","details":"Details2"}]` + "\n"
	if got := w.Body.String(); got != want {
		t.Errorf("handler returned unexpected body: got %v want %v", got, want)
	}
}

func TestApp_GetTechnologies_WithDBError(t *testing.T) {
	app := App{d: &MockDb{
		tech: nil,
		err:  errors.New("unknown error"),
	}}

	r, _ := http.NewRequest("GET", "/api/technologies", nil)
	w := httptest.NewRecorder()

	app.GetTechnologies(w, r)

	if w.Code != http.StatusInternalServerError {
		t.Errorf("handler returned wrong status code: got %v want %v", w.Code, http.StatusOK)
	}

}
Repository: Significant-Gravitas/Auto-GPT
File: rnd/autogpt_libs/autogpt_libs/supabase_integration_credentials_store/types.py (Python)

Code before:
    refresh_token_expires_at: Optional[int]  # seconds
    scopes: list[str]
    metadata: dict[str, Any] = Field(default_factory=dict)


class APIKeyCredentials(_BaseCredentials):
    type: Literal["api_key"] = "api_key"
    api_key: SecretStr
    expires_at: Optional[int]  # seconds


Credentials = Annotated[
    OAuth2Credentials | APIKeyCredentials,
    Field(discriminator="type"),
]


class StateToken(BaseModel):
    state: str
    provider: str
    expires_at: int  # Unix timestamp


class UserMetadata(BaseModel):
    integration_credentials: list[Credentials] = Field(default_factory=list)
    state_tokens: list[StateToken] = Field(default_factory=list)


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


Review comment (security):
```suggestion
    integration_oauth_states: list[OAuthState] = Field(default_factory=list)
```

Code after:
    refresh_token_expires_at: Optional[int]  # seconds
    scopes: list[str]
    metadata: dict[str, Any] = Field(default_factory=dict)


class APIKeyCredentials(_BaseCredentials):
    type: Literal["api_key"] = "api_key"
    api_key: SecretStr
    expires_at: Optional[int]  # seconds


Credentials = Annotated[
    OAuth2Credentials | APIKeyCredentials,
    Field(discriminator="type"),
]


class OAuthState(BaseModel):
    token: str
    provider: str
    expires_at: int  # Unix timestamp


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]
Repository: Significant-Gravitas/Auto-GPT
File: rnd/autogpt_server/autogpt_server/data/db.py (Python)

Code before:
import asyncio
import os

from contextlib import asynccontextmanager
from uuid import uuid4

from dotenv import load_dotenv
from prisma import Prisma, errors
from pydantic import BaseModel, Field, field_validator

load_dotenv()

PRISMA_SCHEMA = os.getenv("PRISMA_SCHEMA", "schema.prisma")
os.environ["PRISMA_SCHEMA_PATH"] = PRISMA_SCHEMA

prisma, conn_id = Prisma(auto_register=True), ""


async def connect(call_count=0):
    global conn_id
    if not conn_id:
        conn_id = str(uuid4())

    try:
        print(f"[Prisma-{conn_id}] Acquiring connection..")
        if not prisma.is_connected():
            await prisma.connect()
        print(f"[Prisma-{conn_id}] Connection acquired!")
    except Exception as e:
        if call_count <= 5:
            print(f"[Prisma-{conn_id}] Connection failed: {e}. Retrying now..")
            await asyncio.sleep(call_count)
            await connect(call_count + 1)
        else:
            raise e


async def disconnect():
    if prisma.is_connected():
        print(f"[Prisma-{conn_id}] Releasing connection.")
        await prisma.disconnect()
        print(f"[Prisma-{conn_id}] Connection released.")


Review comment (suggestion):
Basically this is the main change, I hate to admit that I can't fix it. So retrying is the only way I can think of to fix it.

Code after:
import asyncio
import os
from contextlib import asynccontextmanager
from uuid import uuid4

from dotenv import load_dotenv
from prisma import Prisma
from pydantic import BaseModel, Field, field_validator

load_dotenv()

PRISMA_SCHEMA = os.getenv("PRISMA_SCHEMA", "schema.prisma")
os.environ["PRISMA_SCHEMA_PATH"] = PRISMA_SCHEMA

prisma, conn_id = Prisma(auto_register=True), ""


async def connect(call_count=0):
    global conn_id
    if not conn_id:
        conn_id = str(uuid4())

    try:
        print(f"[Prisma-{conn_id}] Acquiring connection..")
        if not prisma.is_connected():
            await prisma.connect()
        print(f"[Prisma-{conn_id}] Connection acquired!")
    except Exception as e:
        if call_count <= 5:
            print(f"[Prisma-{conn_id}] Connection failed: {e}. Retrying now..")
            await asyncio.sleep(call_count)
            await connect(call_count + 1)
        else:
            raise e


async def disconnect():
    if prisma.is_connected():
        print(f"[Prisma-{conn_id}] Releasing connection.")
        await prisma.disconnect()
        print(f"[Prisma-{conn_id}] Connection released.")
Repository: Significant-Gravitas/Auto-GPT
File: rnd/autogpt_server/autogpt_server/util/decorator.py (Python)

Code before:


class TimingInfo(BaseModel):
    cpu_time: float
    wall_time: float


def _start_measurement() -> Tuple[float, float]:
    return time.time(), os.times()[0] + os.times()[1]


def _end_measurement(
    start_wall_time: float, start_cpu_time: float
) -> Tuple[float, float]:
    end_wall_time = time.time()
    end_cpu_time = os.times()[0] + os.times()[1]
    return end_wall_time - start_wall_time, end_cpu_time - start_cpu_time


P = ParamSpec("P")
T = TypeVar("T")

logger = logging.getLogger(__name__)


def time_measured(func: Callable[P, T]) -> Callable[P, Tuple[TimingInfo, T]]:
    """
    Decorator to measure the time taken by a function to execute.
    """

    @functools.wraps(func)
    def wrapper(*args, **kwargs) -> Tuple[TimingInfo, T]:
        start_wall_time, start_cpu_time = _start_measurement()
        try:
            result = func(*args, **kwargs)
        finally:
            wall_duration, cpu_duration = _end_measurement(
                start_wall_time, start_cpu_time
            )
            timing_info = TimingInfo(cpu_time=cpu_duration, wall_time=wall_duration)
        return timing_info, result

    return wrapper


def error_logged(func: Callable[P, T]) -> Callable[P, T | None]:
    """
    Decorator to suppress and log any exceptions raised by a function.
    """

    @functools.wraps(func)

Review comment (suggestion):
don't forget to also:
```diff
-    def wrapper(*args, **kwargs) -> Tuple[TimingInfo, T]:
+    def wrapper(*args: P.args, **kwargs: P.kwargs) -> Tuple[TimingInfo, T]:
```
otherwise the type checker complains

Code after:


class TimingInfo(BaseModel):
    cpu_time: float
    wall_time: float


def _start_measurement() -> Tuple[float, float]:
    return time.time(), os.times()[0] + os.times()[1]


def _end_measurement(
    start_wall_time: float, start_cpu_time: float
) -> Tuple[float, float]:
    end_wall_time = time.time()
    end_cpu_time = os.times()[0] + os.times()[1]
    return end_wall_time - start_wall_time, end_cpu_time - start_cpu_time


P = ParamSpec("P")
T = TypeVar("T")

logger = logging.getLogger(__name__)


def time_measured(func: Callable[P, T]) -> Callable[P, Tuple[TimingInfo, T]]:
    """
    Decorator to measure the time taken by a function to execute.
    """

    @functools.wraps(func)
    def wrapper(*args: P.args, **kwargs: P.kwargs) -> Tuple[TimingInfo, T]:
        start_wall_time, start_cpu_time = _start_measurement()
        try:
            result = func(*args, **kwargs)
        finally:
            wall_duration, cpu_duration = _end_measurement(
                start_wall_time, start_cpu_time
            )
            timing_info = TimingInfo(cpu_time=cpu_duration, wall_time=wall_duration)
        return timing_info, result

    return wrapper


def error_logged(func: Callable[P, T]) -> Callable[P, T | None]:
    """
    Decorator to suppress and log any exceptions raised by a function.
    """

    @functools.wraps(func)
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/backend/backend/data/db.py (Python)

Code before:
import logging
import os
from contextlib import asynccontextmanager
from uuid import uuid4

from dotenv import load_dotenv
from prisma import Prisma
from pydantic import BaseModel, Field, field_validator

from backend.util.retry import logged_retry

load_dotenv()

PRISMA_SCHEMA = os.getenv("PRISMA_SCHEMA", "schema.prisma")
os.environ["PRISMA_SCHEMA_PATH"] = PRISMA_SCHEMA

prisma, conn_id = Prisma(auto_register=True), ""

logger = logging.getLogger(__name__)


async def connect():
    if prisma.is_connected():
        return

    global conn_id

Review comment (suggestion):
No change here, just connection code tidying.
I decoupled the retry logic and moved it to retry.py.

Code after:
import logging
import os
from contextlib import asynccontextmanager
from uuid import uuid4

from dotenv import load_dotenv
from prisma import Prisma
from pydantic import BaseModel, Field, field_validator

from backend.util.retry import conn_retry

load_dotenv()

PRISMA_SCHEMA = os.getenv("PRISMA_SCHEMA", "schema.prisma")
os.environ["PRISMA_SCHEMA_PATH"] = PRISMA_SCHEMA

prisma = Prisma(auto_register=True)

logger = logging.getLogger(__name__)


@conn_retry("Prisma", "Acquiring connection")
async def connect():
    if prisma.is_connected():
        return
    await prisma.connect()
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/backend/backend/data/redis.py (Python)

Code before:
import logging
import os

from dotenv import load_dotenv
from redis import Redis

from backend.util.retry import conn_retry

load_dotenv()

HOST = os.getenv("REDIS_HOST", "localhost")
PORT = int(os.getenv("REDIS_PORT", "6379"))
PASSWORD = os.getenv("REDIS_PASSWORD", "password")
QUEUE_NAME = os.getenv("REDIS_QUEUE", "execution_events")

logger = logging.getLogger(__name__)
connection: Redis | None = None


@conn_retry
def connect() -> Redis:
    global connection
    if connection:
        return connection

    c = Redis(

Review comment (suggestion):
similar to db.py. This forces a single Redis connection per process.
The redis connection used here is sync, fast API (if it even need redis) will do it separately anyway.

Code after:
import logging
import os

from dotenv import load_dotenv
from redis import Redis

from backend.util.retry import conn_retry

load_dotenv()

HOST = os.getenv("REDIS_HOST", "localhost")
PORT = int(os.getenv("REDIS_PORT", "6379"))
PASSWORD = os.getenv("REDIS_PASSWORD", "password")
QUEUE_NAME = os.getenv("REDIS_QUEUE", "execution_events")

logger = logging.getLogger(__name__)
connection: Redis | None = None


@conn_retry("Redis", "Acquiring connection")
def connect() -> Redis:
    global connection
    if connection:
        return connection

    c = Redis(
Repository: Significant-Gravitas/Auto-GPT
File: autogpt_platform/backend/backend/blocks/iteration.py (Python)

Code before:
from typing import Any, Dict, List, Union

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


class ListIteratorBlock(Block):
    class Input(BlockSchema):
        items: Union[List[Any], Dict[Any, Any]] = 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")

    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, {"key": "value", "key2": "value2"}]},
            test_output=[
                ("item", 1),
                ("item", 2),
                ("item", 3),
                ("item", {"key": "value", "key2": "value2"}),
            ],
            test_mock={},
        )

    def run(self, input_data: Input, **kwargs) -> BlockOutput:

Review comment (suggestion):
Let's not make this more complicated than needed:
```suggestion
        items: list | dict = SchemaField(
```
- `Dict` and `List` and their imports can be replaced by built-in `dict` and `list` which are supported in all python versions we support
- Instead of `Union` we generally use `|`

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),
                ("item", 3),
                ("key", 2),