Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions hindsight-api-slim/hindsight_api/api/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -3465,7 +3465,7 @@ async def metrics_endpoint():
async def api_graph(
bank_id: str,
type: str | None = None,
limit: int = 1000,
limit: int = Query(default=1000, ge=0),
q: str | None = None,
tags: list[str] | None = Query(None),
tags_match: str = "all_strict",
Expand Down Expand Up @@ -3513,8 +3513,8 @@ async def api_list(
consolidation_state: str | None = None,
state: str | None = None,
document_id: str | None = None,
limit: int = 100,
offset: int = 0,
limit: int = Query(default=100, ge=0),
offset: int = Query(default=0, ge=0),
request_context: RequestContext = Depends(get_request_context),
):
"""
Expand Down Expand Up @@ -4248,8 +4248,8 @@ async def api_memories_timeseries(
)
async def api_list_entities(
bank_id: str,
limit: int = Query(default=100, description="Maximum number of entities to return"),
offset: int = Query(default=0, description="Offset for pagination"),
limit: int = Query(default=100, ge=0, description="Maximum number of entities to return"),
offset: int = Query(default=0, ge=0, description="Offset for pagination"),
request_context: RequestContext = Depends(get_request_context),
):
"""List entities for a memory bank with pagination."""
Expand Down Expand Up @@ -4284,7 +4284,7 @@ async def api_list_entities(
)
async def api_entity_graph(
bank_id: str,
limit: int = Query(default=1000, description="Maximum number of co-occurrence edges to return"),
limit: int = Query(default=1000, ge=0, description="Maximum number of co-occurrence edges to return"),
min_count: int = Query(default=1, description="Minimum cooccurrence_count to include an edge"),
request_context: RequestContext = Depends(get_request_context),
):
Expand Down Expand Up @@ -4911,8 +4911,8 @@ async def api_list_documents(
tags_match: str = Query(
"any_strict", description="How to match tags: 'any', 'all', 'any_strict', 'all_strict'"
),
limit: int = 100,
offset: int = 0,
limit: int = Query(default=100, ge=0),
offset: int = Query(default=0, ge=0),
request_context: RequestContext = Depends(get_request_context),
):
"""
Expand Down Expand Up @@ -5096,8 +5096,8 @@ async def api_list_tags(
default="memories",
description="Where to read tags from: 'memories' (memory_units, default) or 'mental_models'.",
),
limit: int = Query(default=100, description="Maximum number of tags to return"),
offset: int = Query(default=0, description="Offset for pagination"),
limit: int = Query(default=100, ge=0, description="Maximum number of tags to return"),
offset: int = Query(default=0, ge=0, description="Offset for pagination"),
request_context: RequestContext = Depends(get_request_context),
):
"""
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
"""Regression: user-facing GET list endpoints must reject negative limit/offset
with a clean 422 at the FastAPI boundary instead of letting the value reach
Postgres (``LIMIT/OFFSET must not be negative``) and surfacing as an opaque 500
that also leaks the raw Postgres error string.

This makes pagination validation consistent with the sibling list endpoints in
the same router (document-chunks / directives / async-ops / audit) that already
declare ``Query(..., ge=...)``. The engine emits ``LIMIT $n OFFSET $n`` with no
``max(0, ...)`` clamp, so the guard has to live at the request boundary.
"""

import uuid

import httpx
import pytest
import pytest_asyncio

from hindsight_api import RequestContext
from hindsight_api.api import create_app


@pytest_asyncio.fixture
async def api_client(memory):
app = create_app(memory, initialize_memory=False)
transport = httpx.ASGITransport(app=app)
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
yield client


def _url(bank_id: str, suffix: str) -> str:
return f"/v1/default/banks/{bank_id}/{suffix}"


# Endpoints that accept a ``limit`` query param.
LIMIT_ENDPOINTS = [
"graph",
"memories/list",
"entities",
"entities/graph",
"documents",
"tags",
]

# Subset that also accept an ``offset`` query param.
OFFSET_ENDPOINTS = [
"memories/list",
"entities",
"documents",
"tags",
]


@pytest.mark.asyncio
@pytest.mark.parametrize("suffix", LIMIT_ENDPOINTS)
async def test_negative_limit_returns_422_not_500(api_client, suffix):
bank_id = f"pag-{uuid.uuid4().hex[:8]}"
resp = await api_client.get(_url(bank_id, suffix), params={"limit": -1})
# FastAPI validation runs before the handler / DB, so a bad pagination input
# is a clean 422 — never a 500 leaking the raw Postgres error.
assert resp.status_code == 422, resp.text


@pytest.mark.asyncio
@pytest.mark.parametrize("suffix", OFFSET_ENDPOINTS)
async def test_negative_offset_returns_422_not_500(api_client, suffix):
bank_id = f"pag-{uuid.uuid4().hex[:8]}"
resp = await api_client.get(_url(bank_id, suffix), params={"offset": -1})
assert resp.status_code == 422, resp.text


@pytest.mark.asyncio
@pytest.mark.parametrize("limit", [0, 1, 100])
async def test_valid_limit_is_accepted_including_zero(api_client, memory, limit):
# Positive control: ge=0 rejects only NEGATIVE limits. A non-negative limit
# — including limit=0 (a valid empty page, LIMIT 0) — must still be accepted,
# so this fix does not change behavior for any previously-valid input.
bank_id = f"pag-{uuid.uuid4().hex[:8]}"
await memory.get_bank_profile(bank_id=bank_id, request_context=RequestContext())
resp = await api_client.get(
_url(bank_id, "memories/list"), params={"limit": limit, "offset": 0}
)
assert resp.status_code == 200, resp.text
Loading