fix: cap explicit page sizes and stop printing query results

app/db/queries.py

@@ -1,7 +1,5 @@
 from typing import Any, Dict, List, Tuple
 
-import rich
-
 from app.db.database import Database
 from app.models.oed_data import OEDColumn
 from app.models.query_params import OEDDataQueryParams
@@ -140,7 +138,6 @@ async def get_filtered_data(
 
     # Execute the query
     records = await db.fetch(query, *query_args)
-    rich.print(f"{records=}")
     return records
 
 

app/main.py

@@ -46,10 +46,11 @@ async def lifespan(app: FastAPI):
 
 ### Technical Note: Automatic Pagination
 
-When a query would return more than {settings.AUTO_PAGINATION_THRESHOLD} records and no explicit limit is
-provided, the API will automatically paginate results to return {settings.AUTO_PAGINATION_THRESHOLD} records
-at a time. The response will include pagination metadata with links to navigate
-to next and previous pages.
+When a query would return more than {settings.AUTO_PAGINATION_THRESHOLD} records and no explicit limit, or one of
+at least {settings.AUTO_PAGINATION_THRESHOLD}, is provided, the API will automatically paginate results to return
+{settings.AUTO_PAGINATION_THRESHOLD} records at a time. A JSON response will include pagination metadata with links to
+navigate to next and previous pages. A CSV response carries the same links in a `Link` header, and the total
+number of matching records in an `X-Total-Count` header.
 
 ## Data Download
 

app/routers/data.py

@@ -94,7 +94,8 @@ async def get_data(
 The response format can be either JSON (default) or CSV.
 
 Results are automatically paginated when they exceed the configured threshold, unless
-an explicit limit is provided.
+an explicit limit below the threshold is provided. CSV responses carry the pagination
+links in a `Link` header and the total in an `X-Total-Count` header.
 
 ### URL examples
 
@@ -192,8 +193,13 @@ async def get_data(
         # Get total count for the query (without pagination)
         total_count = await get_total_count(db, params)
 
-        # Apply automatic pagination if results exceed threshold and no explicit limit provided
-        if total_count > settings.AUTO_PAGINATION_THRESHOLD and params.limit is None:
+        # Apply automatic pagination if results exceed threshold and no explicit limit,
+        # or one at least as large, was provided. Without the cap one request could load
+        # the whole table into memory. ">=" because the `next` links below carry
+        # limit=threshold, and those pages need links of their own.
+        if total_count > settings.AUTO_PAGINATION_THRESHOLD and (
+            params.limit is None or params.limit >= settings.AUTO_PAGINATION_THRESHOLD
+        ):
             params.auto_paginated = True
             params.limit = settings.AUTO_PAGINATION_THRESHOLD
             logger.info(
@@ -203,6 +209,46 @@ async def get_data(
         # Get data from database (now with potential auto-pagination applied)
         data = await get_filtered_data(db, params)
 
+        # Pagination links, for both response formats
+        next_url = None
+        previous_url = None
+        if params.auto_paginated and request:
+            base_url = str(request.url).split("?")[0]
+
+            # Prepare query parameters for pagination links
+            # For Pydantic v2 compatibility
+            query_params = {
+                k: v
+                for k, v in params.model_dump().items()
+                if k not in ["auto_paginated", "offset", "limit"]
+                and v is not None
+            }
+
+            # Set format explicitly if it was provided
+            if params.format != ResponseFormat.JSON:
+                query_params["format"] = params.format
+
+            current_offset = params.offset or 0
+            current_limit = params.limit or total_count
+
+            # Calculate next page link if there are more records
+            if current_offset + current_limit < total_count:
+                next_params = {
+                    **query_params,
+                    "offset": current_offset + current_limit,
+                    "limit": current_limit,
+                }
+                next_url = f"{base_url}?{urlencode(next_params, doseq=True)}"
+
+            # Calculate previous page link if not on first page
+            if current_offset > 0:
+                prev_params = {
+                    **query_params,
+                    "offset": max(0, current_offset - current_limit),
+                    "limit": current_limit,
+                }
+                previous_url = f"{base_url}?{urlencode(prev_params, doseq=True)}"
+
         # Handle response format
         if params.format == ResponseFormat.CSV:
             # Create CSV response
@@ -224,12 +270,25 @@ async def get_data(
             for row in data:
                 writer.writerow(row)
 
+            # A CSV body can't carry pagination metadata, so it goes in headers
+            headers = {
+                "Content-Disposition": "attachment; filename=oed_data.csv",
+                "X-Total-Count": str(total_count),
+            }
+            links = []
+            if next_url:
+                links.append(f'<{next_url}>; rel="next"')
+            if previous_url:
+                links.append(f'<{previous_url}>; rel="prev"')
+            if links:
+                headers["Link"] = ", ".join(links)
+
             # Return streaming response
             output.seek(0)
             return StreamingResponse(
                 iter([output.getvalue()]),
                 media_type="text/csv",
-                headers={"Content-Disposition": "attachment; filename=oed_data.csv"},
+                headers=headers,
             )
         else:
             # JSON response with enhanced pagination info
@@ -240,54 +299,13 @@ async def get_data(
                 "data": data,
             }
 
-            # Add pagination links if automatic pagination was applied
             if params.auto_paginated:
                 # Add flag indicating automatic pagination was applied
                 response["auto_paginated"] = True
-
-                if request:
-                    base_url = str(request.url).split("?")[0]
-
-                    # Prepare query parameters for pagination links
-                    # For Pydantic v2 compatibility
-                    query_params = {
-                        k: v
-                        for k, v in params.model_dump().items()
-                        if k not in ["auto_paginated", "offset", "limit"]
-                        and v is not None
-                    }
-
-                    # Set format explicitly if it was provided
-                    if params.format != ResponseFormat.JSON:
-                        query_params["format"] = params.format
-
-                    # Calculate next page link if there are more records
-                    current_offset = params.offset or 0
-                    current_limit = params.limit or total_count
-                    if current_offset + current_limit < total_count:
-                        next_offset = current_offset + current_limit
-                        next_params = {
-                            **query_params,
-                            "offset": next_offset,
-                            "limit": current_limit,
-                        }
-                        response["next"] = (
-                            f"{base_url}?{urlencode(next_params, doseq=True)}"
-                        )
-
-                    # Calculate previous page link if not on first page
-                    current_offset = params.offset or 0
-                    current_limit = params.limit or total_count
-                    if current_offset > 0:
-                        prev_offset = max(0, current_offset - current_limit)
-                        prev_params = {
-                            **query_params,
-                            "offset": prev_offset,
-                            "limit": current_limit,
-                        }
-                        response["previous"] = (
-                            f"{base_url}?{urlencode(prev_params, doseq=True)}"
-                        )
+                if next_url:
+                    response["next"] = next_url
+                if previous_url:
+                    response["previous"] = previous_url
 
             return response
 

tests/test_auto_pagination.py

@@ -1,3 +1,5 @@
+import re
+
 import pytest
 from fastapi.testclient import TestClient
 
@@ -113,8 +115,108 @@ async def test_auto_pagination_not_needed(monkeypatch, test_client, mock_db):
 
 
 @pytest.mark.asyncio
-async def test_pagination_navigation(monkeypatch):
-    """Test navigation through paginated results."""
-    # Skip this test for now - we'll address it separately
-    # This test requires more complex mocking of the request/response cycle
-    pytest.skip("This test will be implemented in a separate PR")
+@pytest.mark.parametrize(
+    "mock_db", 
+    [
+        {
+            "fetchval": lambda self, query, *args: 25,
+            "fetch": lambda self, query, *args: [{"id": i, "ec": f"1.1.1.{i}", "organism": f"Test Organism {i}"} for i in range(10)]
+        }
+    ], 
+    indirect=True
+)
+async def test_pagination_navigation(monkeypatch, test_client, mock_db):
+    """Test that following `next` reaches the last page.
+
+    Every page after the first is requested with an explicit limit equal to the
+    threshold, so it must still be treated as paginated and carry its own links."""
+    monkeypatch.setattr(settings, "AUTO_PAGINATION_THRESHOLD", 10)
+
+    offsets = []
+    url = "/api/v1/data"
+    while url:
+        data = test_client.get(url).json()
+        offsets.append(data["offset"])
+        url = data.get("next", "").replace("http://testserver", "")
+
+    assert offsets == [0, 10, 20]
+    assert "offset=10" in data["previous"]
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "mock_db", 
+    [
+        {
+            "fetchval": lambda self, query, *args: 100,
+            "fetch": lambda self, query, *args: [{"id": i, "ec": f"1.1.1.{i}", "organism": f"Test Organism {i}"} for i in range(10)]
+        }
+    ], 
+    indirect=True
+)
+async def test_explicit_limit_above_threshold_is_capped(monkeypatch, test_client, mock_db, sql_capture):
+    """Test that an explicit limit larger than the threshold is reduced to the threshold.
+
+    Without the cap, one request can load the whole table into memory."""
+    monkeypatch.setattr(settings, "AUTO_PAGINATION_THRESHOLD", 10)
+
+    response = test_client.get("/api/v1/data?limit=1000")
+
+    assert response.status_code == 200
+    data = response.json()
+
+    # The query itself is limited, not just the response
+    data_query = [q["query"] for q in sql_capture if "COUNT(*)" not in q["query"]][0]
+    assert re.search(r"LIMIT 10\b", data_query)
+
+    assert data["auto_paginated"] is True
+    assert data["limit"] == 10
+    assert data["total"] == 100
+    assert "offset=10" in data["next"]
+    assert "limit=10" in data["next"]
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "mock_db", 
+    [
+        {
+            "fetchval": lambda self, query, *args: 100,
+            "fetch": lambda self, query, *args: [{"id": i, "ec": "1.1.1.1", "organism": "Distinctive organism"} for i in range(5)]
+        }
+    ], 
+    indirect=True
+)
+async def test_results_are_not_printed(test_client, mock_db, capsys):
+    """Test that query results are not written to stdout.
+
+    Printing every record of a large result set cost far more memory than the records."""
+    response = test_client.get("/api/v1/data?limit=5")
+
+    assert response.status_code == 200
+    assert "Distinctive organism" not in capsys.readouterr().out
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "mock_db", 
+    [
+        {
+            "fetchval": lambda self, query, *args: 100,
+            "fetch": lambda self, query, *args: [{"id": i, "ec": f"1.1.1.{i}", "organism": f"Test Organism {i}"} for i in range(10)]
+        }
+    ], 
+    indirect=True
+)
+async def test_capped_csv_says_there_is_more(monkeypatch, test_client, mock_db):
+    """Test that a CSV response cut off by the cap says so in its headers.
+
+    A CSV body has nowhere to carry pagination metadata."""
+    monkeypatch.setattr(settings, "AUTO_PAGINATION_THRESHOLD", 10)
+
+    response = test_client.get("/api/v1/data?format=csv&limit=1000")
+
+    assert response.status_code == 200
+    assert response.headers["x-total-count"] == "100"
+    assert 'rel="next"' in response.headers["link"]
+    assert "offset=10" in response.headers["link"]
+    assert "format=csv" in response.headers["link"]