Skip to content

Commit b242d4f

Browse files
perf: Cloud Run CPU optimization, server-side pagination, GCS cache (#4710)
## Summary - **Cloud Run CPU 2→1**: Both frontend (nginx) and backend (uvicorn async) don't need 2 cores. Backend upgraded to gen2 execution environment for better CPU/network performance. - **Server-side pagination**: `/plots/filter` now accepts `limit` and `offset` query params. Counts and totals are still computed from all filtered images. Fully backward-compatible — no params = all images. - **GCS Cache-Control 1h→1d**: All `gsutil cp` commands in impl-generate, impl-merge, and impl-repair workflows now set `Cache-Control: public, max-age=86400`. ## Changed files | File | Change | |------|--------| | `app/cloudbuild.yaml` | CPU 2→1 | | `api/cloudbuild.yaml` | CPU 2→1, add `--execution-environment=gen2` | | `api/schemas.py` | Add `offset`/`limit` fields to `FilteredPlotsResponse` | | `api/routers/plots.py` | Add pagination query params, cache key update, slice logic | | `tests/unit/api/test_routers.py` | 4 new pagination tests + cached mock update | | `.github/workflows/impl-generate.yml` | Cache-Control header on gsutil cp | | `.github/workflows/impl-merge.yml` | Cache-Control header on gsutil cp | | `.github/workflows/impl-repair.yml` | Cache-Control header on gsutil cp | ## Test plan - [x] `uv run ruff check` passes on all changed Python files - [x] `uv run pytest tests/unit/api/test_routers.py` — 101 tests pass - [ ] Verify `/plots/filter?limit=10&offset=0` returns 10 images with correct `total` - [ ] Verify `/plots/filter` without params returns all images (backward compat) - [ ] Verify Cloud Run deploys successfully with new CPU/gen2 settings 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 11bc78c commit b242d4f

File tree

9 files changed

+210
-86
lines changed

9 files changed

+210
-86
lines changed

.github/workflows/impl-generate.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,7 @@ jobs:
534534
535535
# Upload PNG (with watermark)
536536
if [ -f "$IMPL_DIR/plot.png" ]; then
537-
gsutil cp "$IMPL_DIR/plot.png" "${STAGING_PATH}/plot.png"
537+
gsutil -h "Cache-Control:public, max-age=86400" cp "$IMPL_DIR/plot.png" "${STAGING_PATH}/plot.png"
538538
gsutil acl ch -u AllUsers:R "${STAGING_PATH}/plot.png" 2>/dev/null || true
539539
echo "png_url=${PUBLIC_URL}/plot.png" >> $GITHUB_OUTPUT
540540
echo "uploaded=true" >> $GITHUB_OUTPUT
@@ -545,15 +545,15 @@ jobs:
545545
546546
# Upload thumbnail
547547
if [ -f "$IMPL_DIR/plot_thumb.png" ]; then
548-
gsutil cp "$IMPL_DIR/plot_thumb.png" "${STAGING_PATH}/plot_thumb.png"
548+
gsutil -h "Cache-Control:public, max-age=86400" cp "$IMPL_DIR/plot_thumb.png" "${STAGING_PATH}/plot_thumb.png"
549549
gsutil acl ch -u AllUsers:R "${STAGING_PATH}/plot_thumb.png" 2>/dev/null || true
550550
echo "thumb_url=${PUBLIC_URL}/plot_thumb.png" >> $GITHUB_OUTPUT
551551
echo "::notice::Uploaded thumbnail"
552552
fi
553553
554554
# Upload HTML (interactive libraries)
555555
if [ -f "$IMPL_DIR/plot.html" ]; then
556-
gsutil cp "$IMPL_DIR/plot.html" "${STAGING_PATH}/plot.html"
556+
gsutil -h "Cache-Control:public, max-age=86400" cp "$IMPL_DIR/plot.html" "${STAGING_PATH}/plot.html"
557557
gsutil acl ch -u AllUsers:R "${STAGING_PATH}/plot.html" 2>/dev/null || true
558558
echo "html_url=${PUBLIC_URL}/plot.html" >> $GITHUB_OUTPUT
559559
fi

.github/workflows/impl-merge.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ jobs:
201201
PRODUCTION="gs://pyplots-images/plots/${SPEC_ID}/${LIBRARY}"
202202
203203
# Copy from staging to production
204-
gsutil -m cp -r "${STAGING}/*" "${PRODUCTION}/" 2>/dev/null || echo "No staging files to promote"
204+
gsutil -m -h "Cache-Control:public, max-age=86400" cp -r "${STAGING}/*" "${PRODUCTION}/" 2>/dev/null || echo "No staging files to promote"
205205
206206
# Make production files public
207207
gsutil -m acl ch -r -u AllUsers:R "${PRODUCTION}/" 2>/dev/null || true

.github/workflows/impl-repair.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,17 +187,17 @@ jobs:
187187
gcloud auth activate-service-account --key-file=/tmp/gcs-key.json
188188
189189
if [ -f "$IMPL_DIR/plot.png" ]; then
190-
gsutil cp "$IMPL_DIR/plot.png" "${STAGING_PATH}/plot.png"
190+
gsutil -h "Cache-Control:public, max-age=86400" cp "$IMPL_DIR/plot.png" "${STAGING_PATH}/plot.png"
191191
gsutil acl ch -u AllUsers:R "${STAGING_PATH}/plot.png" 2>/dev/null || true
192192
fi
193193
194194
if [ -f "$IMPL_DIR/plot_thumb.png" ]; then
195-
gsutil cp "$IMPL_DIR/plot_thumb.png" "${STAGING_PATH}/plot_thumb.png"
195+
gsutil -h "Cache-Control:public, max-age=86400" cp "$IMPL_DIR/plot_thumb.png" "${STAGING_PATH}/plot_thumb.png"
196196
gsutil acl ch -u AllUsers:R "${STAGING_PATH}/plot_thumb.png" 2>/dev/null || true
197197
fi
198198
199199
if [ -f "$IMPL_DIR/plot.html" ]; then
200-
gsutil cp "$IMPL_DIR/plot.html" "${STAGING_PATH}/plot.html"
200+
gsutil -h "Cache-Control:public, max-age=86400" cp "$IMPL_DIR/plot.html" "${STAGING_PATH}/plot.html"
201201
gsutil acl ch -u AllUsers:R "${STAGING_PATH}/plot.html" 2>/dev/null || true
202202
fi
203203

api/cloudbuild.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ substitutions:
33
_SERVICE_NAME: pyplots-backend
44
_REGION: europe-west4
55
_MEMORY: 512Mi
6-
_CPU: "2"
6+
_CPU: "1"
77
_MIN_INSTANCES: "1"
88
_MAX_INSTANCES: "3"
99

@@ -55,6 +55,7 @@ steps:
5555
- "--add-cloudsql-instances=pyplots:europe-west4:pyplots-db"
5656
- "--set-secrets=DATABASE_URL=DATABASE_URL:latest"
5757
- "--set-env-vars=ENVIRONMENT=production"
58+
- "--execution-environment=gen2"
5859
- "--set-env-vars=GOOGLE_CLOUD_PROJECT=$PROJECT_ID"
5960
- "--set-env-vars=GCS_BUCKET=pyplots-images"
6061
id: "deploy"

api/routers/plots.py

Lines changed: 66 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import logging
44
from collections.abc import Callable
55

6-
from fastapi import APIRouter, Depends, Request
6+
from fastapi import APIRouter, Depends, Query, Request
77
from sqlalchemy.exc import SQLAlchemyError
88
from sqlalchemy.ext.asyncio import AsyncSession
99

@@ -267,6 +267,8 @@ def _build_cache_key(filter_groups: list[dict]) -> str:
267267
"""
268268
Build cache key from filter groups.
269269
270+
Groups are sorted by category so key is stable regardless of query param order.
271+
270272
Args:
271273
filter_groups: List of filter group dicts
272274
@@ -276,7 +278,8 @@ def _build_cache_key(filter_groups: list[dict]) -> str:
276278
if not filter_groups:
277279
return "filter:all"
278280

279-
cache_parts = [f"{g['category']}={','.join(sorted(g['values']))}" for g in filter_groups]
281+
normalized = sorted(filter_groups, key=lambda g: g["category"])
282+
cache_parts = [f"{g['category']}={','.join(sorted(g['values']))}" for g in normalized]
280283
return f"filter:{':'.join(cache_parts)}"
281284

282285

@@ -370,7 +373,12 @@ def _filter_images(
370373

371374

372375
@router.get("/plots/filter", response_model=FilteredPlotsResponse)
373-
async def get_filtered_plots(request: Request, db: AsyncSession = Depends(require_db)):
376+
async def get_filtered_plots(
377+
request: Request,
378+
db: AsyncSession = Depends(require_db),
379+
limit: int | None = Query(None, ge=1),
380+
offset: int = Query(0, ge=0),
381+
):
374382
"""
375383
Get filtered plot images with counts for all filter categories.
376384
@@ -398,55 +406,65 @@ async def get_filtered_plots(request: Request, db: AsyncSession = Depends(requir
398406
# Parse query parameters
399407
filter_groups = _parse_filter_groups(request)
400408

401-
# Check cache
409+
# Check cache (cache stores unpaginated result; pagination applied after)
402410
cache_key = _build_cache_key(filter_groups)
411+
cached: FilteredPlotsResponse | None = None
403412
try:
404413
cached = get_cache(cache_key)
405-
if cached:
406-
return cached
407414
except Exception as e:
408-
# Cache failures are non-fatal, log and continue
409415
logger.warning("Cache read failed for key %s: %s", cache_key, e)
410416

411-
# Fetch data from database
412-
try:
413-
repo = SpecRepository(db)
414-
all_specs = await repo.get_all()
415-
except SQLAlchemyError as e:
416-
logger.error("Database query failed in get_filtered_plots: %s", e)
417-
raise DatabaseQueryError("fetch_specs", str(e)) from e
418-
419-
# Build data structures
420-
spec_lookup = _build_spec_lookup(all_specs)
421-
impl_lookup = _build_impl_lookup(all_specs)
422-
all_images = _collect_all_images(all_specs)
423-
spec_id_to_tags = {spec_id: spec_data["tags"] for spec_id, spec_data in spec_lookup.items()}
424-
425-
# Filter images
426-
filtered_images = _filter_images(all_images, filter_groups, spec_lookup, impl_lookup)
427-
428-
# Calculate counts
429-
global_counts = _calculate_global_counts(all_specs)
430-
counts = _calculate_contextual_counts(filtered_images, spec_id_to_tags, impl_lookup)
431-
or_counts = _calculate_or_counts(filter_groups, all_images, spec_id_to_tags, spec_lookup, impl_lookup)
432-
433-
# Build spec_id -> title mapping for search/tooltips
434-
spec_titles = {spec_id: data["spec"].title for spec_id, data in spec_lookup.items() if data["spec"].title}
435-
436-
# Build and cache response
437-
result = FilteredPlotsResponse(
438-
total=len(filtered_images),
439-
images=filtered_images,
440-
counts=counts,
441-
globalCounts=global_counts,
442-
orCounts=or_counts,
443-
specTitles=spec_titles,
417+
if cached is None:
418+
# Fetch data from database
419+
try:
420+
repo = SpecRepository(db)
421+
all_specs = await repo.get_all()
422+
except SQLAlchemyError as e:
423+
logger.error("Database query failed in get_filtered_plots: %s", e)
424+
raise DatabaseQueryError("fetch_specs", str(e)) from e
425+
426+
# Build data structures
427+
spec_lookup = _build_spec_lookup(all_specs)
428+
impl_lookup = _build_impl_lookup(all_specs)
429+
all_images = _collect_all_images(all_specs)
430+
spec_id_to_tags = {spec_id: spec_data["tags"] for spec_id, spec_data in spec_lookup.items()}
431+
432+
# Filter images
433+
filtered_images = _filter_images(all_images, filter_groups, spec_lookup, impl_lookup)
434+
435+
# Calculate counts (always from ALL filtered images, not paginated)
436+
global_counts = _calculate_global_counts(all_specs)
437+
counts = _calculate_contextual_counts(filtered_images, spec_id_to_tags, impl_lookup)
438+
or_counts = _calculate_or_counts(filter_groups, all_images, spec_id_to_tags, spec_lookup, impl_lookup)
439+
440+
# Build spec_id -> title mapping for search/tooltips
441+
spec_titles = {spec_id: data["spec"].title for spec_id, data in spec_lookup.items() if data["spec"].title}
442+
443+
# Cache the full (unpaginated) result
444+
cached = FilteredPlotsResponse(
445+
total=len(filtered_images),
446+
images=filtered_images,
447+
counts=counts,
448+
globalCounts=global_counts,
449+
orCounts=or_counts,
450+
specTitles=spec_titles,
451+
)
452+
453+
try:
454+
set_cache(cache_key, cached)
455+
except Exception as e:
456+
logger.warning("Cache write failed for key %s: %s", cache_key, e)
457+
458+
# Apply pagination on top of (possibly cached) result
459+
paginated = cached.images[offset : offset + limit] if limit else cached.images[offset:]
460+
461+
return FilteredPlotsResponse(
462+
total=cached.total,
463+
images=paginated,
464+
counts=cached.counts,
465+
globalCounts=cached.globalCounts,
466+
orCounts=cached.orCounts,
467+
specTitles=cached.specTitles,
468+
offset=offset,
469+
limit=limit,
444470
)
445-
446-
try:
447-
set_cache(cache_key, result)
448-
except Exception as e:
449-
# Cache failures are non-fatal, log and continue
450-
logger.warning("Cache write failed for key %s: %s", cache_key, e)
451-
452-
return result

api/schemas.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ class FilteredPlotsResponse(BaseModel):
9898
globalCounts: dict[str, dict[str, int]] # Same structure for global counts
9999
orCounts: list[dict[str, int]] # Per-group OR counts
100100
specTitles: dict[str, str] = {} # Mapping spec_id -> title for search/tooltips
101+
offset: int = 0
102+
limit: int | None = None
101103

102104

103105
class LibraryInfo(BaseModel):

app/cloudbuild.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ steps:
4242
- "--memory"
4343
- "256Mi"
4444
- "--cpu"
45-
- "2"
45+
- "1"
4646
- "--timeout"
4747
- "60"
4848
- "--min-instances"

tests/unit/api/mcp/test_tools.py

Lines changed: 21 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,15 @@
99
import pytest
1010

1111
# Import the tool functions from the module
12-
# Note: These are FunctionTool objects, we need to access .fn to get the actual callable
13-
from api.mcp.server import get_implementation as get_implementation_tool
14-
from api.mcp.server import get_spec_detail as get_spec_detail_tool
15-
from api.mcp.server import get_tag_values as get_tag_values_tool
16-
from api.mcp.server import list_libraries as list_libraries_tool
17-
from api.mcp.server import list_specs as list_specs_tool
18-
from api.mcp.server import search_specs_by_tags as search_specs_by_tags_tool
19-
20-
21-
# Extract the actual functions from the FunctionTool wrappers
22-
list_specs = list_specs_tool.fn
23-
search_specs_by_tags = search_specs_by_tags_tool.fn
24-
get_spec_detail = get_spec_detail_tool.fn
25-
get_implementation = get_implementation_tool.fn
26-
list_libraries = list_libraries_tool.fn
27-
get_tag_values = get_tag_values_tool.fn
12+
# With FastMCP's @mcp_server.tool() decorator, these are plain async functions
13+
from api.mcp.server import (
14+
get_implementation,
15+
get_spec_detail,
16+
get_tag_values,
17+
list_libraries,
18+
list_specs,
19+
search_specs_by_tags,
20+
)
2821

2922

3023
@pytest.fixture
@@ -358,7 +351,8 @@ async def test_all_tools_registered(self):
358351
"""MCP server should have all 6 tools registered."""
359352
from api.mcp.server import mcp_server
360353

361-
tool_names = await mcp_server.get_tools()
354+
tools = await mcp_server.list_tools()
355+
tool_names = {t.name for t in tools}
362356
expected = {
363357
"list_specs",
364358
"search_specs_by_tags",
@@ -367,31 +361,29 @@ async def test_all_tools_registered(self):
367361
"list_libraries",
368362
"get_tag_values",
369363
}
370-
assert set(tool_names) == expected
364+
assert tool_names == expected
371365

372366
@pytest.mark.asyncio
373367
async def test_tool_objects_have_correct_type(self):
374368
"""Each registered tool should be a FunctionTool with a callable fn."""
375369
from api.mcp.server import mcp_server
376370

377-
tool_names = await mcp_server.get_tools()
378-
for name in tool_names:
379-
tool = await mcp_server.get_tool(name)
380-
assert hasattr(tool, "fn"), f"Tool {name} has no 'fn' attribute"
381-
assert callable(tool.fn), f"Tool {name}.fn is not callable"
371+
tools = await mcp_server.list_tools()
372+
for tool in tools:
373+
assert hasattr(tool, "fn"), f"Tool {tool.name} has no 'fn' attribute"
374+
assert callable(tool.fn), f"Tool {tool.name}.fn is not callable"
382375

383376
@pytest.mark.asyncio
384377
async def test_tool_schemas_are_valid(self):
385378
"""Each tool should have a valid JSON Schema for its parameters."""
386379
from api.mcp.server import mcp_server
387380

388-
tool_names = await mcp_server.get_tools()
389-
for name in tool_names:
390-
tool = await mcp_server.get_tool(name)
381+
tools = await mcp_server.list_tools()
382+
for tool in tools:
391383
schema = tool.parameters
392-
assert isinstance(schema, dict), f"Tool {name} schema is not a dict"
393-
assert "properties" in schema, f"Tool {name} schema has no 'properties'"
394-
assert schema.get("type") == "object", f"Tool {name} schema type is not 'object'"
384+
assert isinstance(schema, dict), f"Tool {tool.name} schema is not a dict"
385+
assert "properties" in schema, f"Tool {tool.name} schema has no 'properties'"
386+
assert schema.get("type") == "object", f"Tool {tool.name} schema type is not 'object'"
395387

396388
@pytest.mark.asyncio
397389
async def test_get_tag_values_via_call_tool(self):

0 commit comments

Comments
 (0)