diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 5ba8a604a9..72c23a1976 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -58,4 +58,3 @@ Fixes # Backend: cd backend && make lint && make test Frontend: cd frontend && pnpm format && pnpm lint && pnpm typecheck && BETTER_AUTH_SECRET=local-dev-secret pnpm build && make test Frontend E2E (if you touched frontend/): cd frontend && make test-e2e --> - diff --git a/backend/app/gateway/pagination.py b/backend/app/gateway/pagination.py new file mode 100644 index 0000000000..a7e14a90bb --- /dev/null +++ b/backend/app/gateway/pagination.py @@ -0,0 +1,20 @@ +"""Shared pagination helpers for gateway routers.""" + +from __future__ import annotations + + +def trim_run_message_page(rows: list[dict], *, limit: int, after_seq: int | None) -> tuple[list[dict], bool]: + """Trim a ``limit + 1`` run message page without dropping visible rows. + + ``rows`` must be ordered oldest to newest. When an extra sentinel row is + present, latest-page and ``before_seq`` pagination receive the sentinel on + the older side, while ``after_seq`` pagination receives it on the newer side. + """ + has_more = len(rows) > limit + if not has_more: + return rows, False + + if after_seq is not None: + return rows[:limit], True + + return rows[-limit:], True diff --git a/backend/app/gateway/routers/runs.py b/backend/app/gateway/routers/runs.py index 1e61ffd25d..a0aae03cd2 100644 --- a/backend/app/gateway/routers/runs.py +++ b/backend/app/gateway/routers/runs.py @@ -15,6 +15,7 @@ from app.gateway.authz import require_permission from app.gateway.deps import get_checkpointer, get_feedback_repo, get_run_event_store, get_run_manager, get_run_store, get_stream_bridge +from app.gateway.pagination import trim_run_message_page from app.gateway.routers.thread_runs import RunCreateRequest from app.gateway.services import sse_consumer, start_run, wait_for_run_completion from deerflow.runtime import serialize_channel_values @@ -129,8 +130,7 @@ async def run_messages( before_seq=before_seq, after_seq=after_seq, ) - has_more = len(rows) > limit - data = rows[:limit] if has_more else rows + data, has_more = trim_run_message_page(rows, limit=limit, after_seq=after_seq) return {"data": data, "has_more": has_more} diff --git a/backend/app/gateway/routers/thread_runs.py b/backend/app/gateway/routers/thread_runs.py index 9fc4dfa683..65bf0698f1 100644 --- a/backend/app/gateway/routers/thread_runs.py +++ b/backend/app/gateway/routers/thread_runs.py @@ -21,6 +21,7 @@ from app.gateway.authz import require_permission from app.gateway.deps import get_checkpointer, get_current_user, get_feedback_repo, get_run_event_store, get_run_manager, get_run_store, get_stream_bridge +from app.gateway.pagination import trim_run_message_page from app.gateway.services import sse_consumer, start_run, wait_for_run_completion from deerflow.runtime import RunRecord, RunStatus, serialize_channel_values @@ -402,8 +403,7 @@ async def list_run_messages( before_seq=before_seq, after_seq=after_seq, ) - has_more = len(rows) > limit - data = rows[:limit] if has_more else rows + data, has_more = trim_run_message_page(rows, limit=limit, after_seq=after_seq) return {"data": data, "has_more": has_more} diff --git a/backend/tests/_run_message_pagination_helpers.py b/backend/tests/_run_message_pagination_helpers.py new file mode 100644 index 0000000000..264a80a519 --- /dev/null +++ b/backend/tests/_run_message_pagination_helpers.py @@ -0,0 +1,16 @@ +from fastapi.testclient import TestClient + + +def assert_run_message_page( + client: TestClient, + url: str, + *, + expected_seq: list[int], + has_more: bool = True, +) -> None: + response = client.get(url) + + assert response.status_code == 200 + body = response.json() + assert body["has_more"] is has_more + assert [m["seq"] for m in body["data"]] == expected_seq diff --git a/backend/tests/test_runs_api_endpoints.py b/backend/tests/test_runs_api_endpoints.py index 1826e4d8ed..dfa5869c88 100644 --- a/backend/tests/test_runs_api_endpoints.py +++ b/backend/tests/test_runs_api_endpoints.py @@ -5,6 +5,7 @@ from unittest.mock import AsyncMock, MagicMock from _router_auth_helpers import make_authed_test_app +from _run_message_pagination_helpers import assert_run_message_page from fastapi.testclient import TestClient from app.gateway.routers import runs @@ -97,6 +98,51 @@ def test_run_messages_has_more_true_when_extra_row_returned(): body = response.json() assert body["has_more"] is True assert len(body["data"]) == 50 # trimmed to limit + assert [m["seq"] for m in body["data"]] == list(range(2, 52)) + + +def test_run_messages_default_page_keeps_newest_messages_when_extra_row_returned(): + """Default latest-page trimming drops the older sentinel row, not the newest message.""" + rows = [_make_message(i) for i in range(16, 67)] + run_record = {"run_id": "run-2", "thread_id": "thread-2"} + app = _make_app( + run_store=_make_run_store(run_record), + event_store=_make_event_store(rows), + ) + with TestClient(app) as client: + assert_run_message_page(client, "/api/runs/run-2/messages", expected_seq=list(range(17, 67))) + + +def test_run_messages_before_seq_page_keeps_newest_side_when_extra_row_returned(): + """Backward pagination trims the older sentinel so adjacent pages do not miss the boundary message.""" + rows = [_make_message(i) for i in range(1, 18)] + run_record = {"run_id": "run-2", "thread_id": "thread-2"} + app = _make_app( + run_store=_make_run_store(run_record), + event_store=_make_event_store(rows), + ) + with TestClient(app) as client: + assert_run_message_page( + client, + "/api/runs/run-2/messages?before_seq=18&limit=16", + expected_seq=list(range(2, 18)), + ) + + +def test_run_messages_after_seq_page_keeps_oldest_side_when_extra_row_returned(): + """Forward pagination still trims the newer sentinel row.""" + rows = [_make_message(i) for i in range(11, 62)] + run_record = {"run_id": "run-2", "thread_id": "thread-2"} + app = _make_app( + run_store=_make_run_store(run_record), + event_store=_make_event_store(rows), + ) + with TestClient(app) as client: + assert_run_message_page( + client, + "/api/runs/run-2/messages?after_seq=10", + expected_seq=list(range(11, 61)), + ) def test_run_messages_passes_after_seq_to_event_store(): diff --git a/backend/tests/test_thread_run_messages_pagination.py b/backend/tests/test_thread_run_messages_pagination.py index 9098e2b73f..6d36001ae9 100644 --- a/backend/tests/test_thread_run_messages_pagination.py +++ b/backend/tests/test_thread_run_messages_pagination.py @@ -6,6 +6,7 @@ from unittest.mock import AsyncMock, MagicMock from _router_auth_helpers import make_authed_test_app +from _run_message_pagination_helpers import assert_run_message_page from fastapi.testclient import TestClient from app.gateway.routers import thread_runs @@ -88,6 +89,43 @@ def test_has_more_true_when_extra_row_returned(): body = response.json() assert body["has_more"] is True assert len(body["data"]) == 50 # trimmed to limit + assert [m["seq"] for m in body["data"]] == list(range(2, 52)) + + +def test_default_page_keeps_newest_messages_when_extra_row_returned(): + """Default latest-page trimming drops the older sentinel row, not the newest message.""" + rows = [_make_message(i) for i in range(16, 67)] + app = _make_app(event_store=_make_event_store(rows)) + with TestClient(app) as client: + assert_run_message_page( + client, + "/api/threads/thread-2/runs/run-2/messages", + expected_seq=list(range(17, 67)), + ) + + +def test_before_seq_page_keeps_newest_side_when_extra_row_returned(): + """Backward pagination trims the older sentinel so adjacent pages do not miss the boundary message.""" + rows = [_make_message(i) for i in range(1, 18)] + app = _make_app(event_store=_make_event_store(rows)) + with TestClient(app) as client: + assert_run_message_page( + client, + "/api/threads/thread-2/runs/run-2/messages?before_seq=18&limit=16", + expected_seq=list(range(2, 18)), + ) + + +def test_after_seq_page_keeps_oldest_side_when_extra_row_returned(): + """Forward pagination still trims the newer sentinel row.""" + rows = [_make_message(i) for i in range(11, 62)] + app = _make_app(event_store=_make_event_store(rows)) + with TestClient(app) as client: + assert_run_message_page( + client, + "/api/threads/thread-2/runs/run-2/messages?after_seq=10", + expected_seq=list(range(11, 61)), + ) def test_after_seq_forwarded_to_event_store():