Apply self-review findings: drop unqualified recycling, fix docs drift

Worker recycling (--max-requests) is removed rather than retuned:
review analysis showed the 500-request budget recycles faster than the
~170s re-import at peak (the counter advances on cheap requests too),
jitter 100 cannot decorrelate two workers, a mid-life re-import runs
without cpu-boost and CPU-throttled at idle, and the qualifying soak
never executed a recycle. The ops doc now records why it needs its own
qualification. Also: pin assertions get boundary anchors; the ops doc's
source-of-truth section acknowledges push.yml job pins; stale cold-start
figures replaced with measured values; the changelog credits staging's
shape change; the min-instances runbook gains WHEN + verification; the
duplicated rationale comments collapse to pointers; load-generator
cleanup (single exception path, Counter, always/never cache-bust, dead
fields removed).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: anth-volk

.github/scripts/cloud_run_env.sh

@@ -15,11 +15,8 @@ cloud_run_set_defaults() {
   CLOUD_RUN_TIMEOUT="${CLOUD_RUN_TIMEOUT:-300}"
   CLOUD_RUN_MIN_INSTANCES="${CLOUD_RUN_MIN_INSTANCES:-0}"
   CLOUD_RUN_MAX_INSTANCES="${CLOUD_RUN_MAX_INSTANCES:-1}"
-  # Runtime shape from the Stage 2 capacity qualification
-  # (docs/migration/history/pr4-stage2-runtime-timing.md). Both values are
-  # pinned explicitly on every deploy: gcloud inherits unspecified template
-  # fields, and `--concurrency default` resolves to the platform default
-  # (640), not the historical 80.
+  # Stage 2-qualified runtime shape, pinned explicitly on every deploy —
+  # rationale in docs/migration/cloud-run-operations.md ("Runtime shape").
   CLOUD_RUN_CONCURRENCY="${CLOUD_RUN_CONCURRENCY:-4}"
   CLOUD_RUN_WEB_CONCURRENCY="${CLOUD_RUN_WEB_CONCURRENCY:-2}"
   CLOUD_RUN_PORT="${CLOUD_RUN_PORT:-8080}"

.github/workflows/push.yml

@@ -542,11 +542,9 @@ jobs:
     environment: production
     env:
       CLOUD_RUN_SERVICE: policyengine-api
-      # Sized by the Stage 2 capacity qualification: peak real traffic ~11
-      # RPS (mostly cached/light); one instance handles ~1-2 concurrent
-      # uncached calculates. Revision-level min-instances stays 0 — warm
-      # capacity is a SERVICE-level setting (revision-level would keep a
-      # warm 16Gi instance per accumulated stage3-* tag).
+      # Sized by the Stage 2 qualification — rationale and numbers in
+      # docs/migration/cloud-run-operations.md ("Runtime shape and scaling").
+      # Never pin a min-instances value here: warm capacity is service-level.
       CLOUD_RUN_MAX_INSTANCES: "4"
     permissions:
       contents: read

changelog.d/pr4-stage3-cloud-run-runtime-config.changed.md

@@ -1 +1 @@
-Configure the production Cloud Run runtime from the Stage 2 capacity qualification: two gunicorn workers, request concurrency 4, production max-instances 4, staging pinned to scale-to-zero, and worker recycling to bound memory growth.
+Configure both Cloud Run services' runtime from the Stage 2 capacity qualification — two gunicorn workers and request concurrency 4 on staging and production alike — plus production max-instances 4 and explicit staging scale-to-zero pins.

docs/migration/cloud-run-operations.md

@@ -5,9 +5,12 @@ edited in place whenever behavior changes. Per-stage records (bootstrap commands
 exit-gate evidence, one-off measurements) are append-only documents under `history/` and
 are never updated to match later reality.
 
-Configuration **values** (CPU, memory, scaling, secrets, service names) live in
-`.github/scripts/cloud_run_env.sh` and the deploy scripts — they are the source of truth
-and are intentionally not duplicated here. This document explains the **semantics**.
+Configuration **values** live in two layers, both source of truth for their scope:
+`.github/scripts/cloud_run_env.sh` holds the defaults (CPU, memory, runtime shape,
+secrets, service names), and **per-job `env:` pins in `.github/workflows/push.yml`
+override them per track** (e.g. production `CLOUD_RUN_MAX_INSTANCES`). To change what a
+track deploys, edit the job pin, not just the default — job pins win. Values are
+intentionally not duplicated here; this document explains the **semantics**.
 
 ## Topology
 
@@ -32,21 +35,24 @@ and are intentionally not duplicated here. This document explains the **semantic
 
 ## Startup behavior
 
-The app import is slow (~230s cold: `policyengine_api/country.py` instantiates five
-country packages, ~210s of it) and Cloud Run hard-caps total startup-probe time at 240s —
-the default TCP probe already sits at that cap, and **no probe configuration can extend
-it**. Two mitigations are therefore built in:
+The app import is slow (measured container boot p50 171.5s / p95 215.2s over 7 days,
+~161–168s on cpu-boosted revisions; dominated by `policyengine_api/country.py`
+instantiating five country packages — see
+[`history/pr4-stage2-runtime-timing.md`](history/pr4-stage2-runtime-timing.md)) and
+Cloud Run hard-caps total startup-probe time at 240s — the default TCP probe already
+sits at that cap, and **no probe configuration can extend it**. Two mitigations are
+therefore built in:
 
 - `gcp/cloud_run/start.sh` runs **gunicorn with `uvicorn.workers.UvicornWorker` and no
   `--preload`**: the gunicorn master binds the listen socket in milliseconds (satisfying
   the startup probe immediately), and the import happens in the worker after fork.
   `--timeout 0` is required — a worker mid-import does not heartbeat, and gunicorn's
   default 30s watchdog would kill it. `--keep-alive 5` and `--forwarded-allow-ips '*'`
-  preserve the previous uvicorn keep-alive and proxy-header behavior.
-  `--max-requests 500` (+jitter 100) recycles workers to bound slow memory growth under
-  sustained load; a recycling worker re-imports for ~3 minutes, halving instance
-  capacity while it does.
-- Candidate deploys set `--cpu-boost` to shorten the import.
+  preserve the previous uvicorn keep-alive and proxy-header behavior. Worker recycling
+  (`--max-requests`) is deliberately NOT enabled: with 2 workers and a multi-minute
+  re-import that runs without cpu-boost (and CPU-throttled at idle), recycling risks
+  correlated zero-ready-worker windows; it needs its own qualification before adoption.
+- Candidate deploys set `--cpu-boost` to shorten the import (instance startup only).
 
 ## Runtime shape and scaling (from the Stage 2 qualification)
 
@@ -74,6 +80,17 @@ Values measured and justified in
     --min-instances 1
   ```
 
+  **When:** once, immediately after the Stage 3 PR merges — before evaluating the
+  Stage 3 exit gates (the idle-readiness gate cannot pass without it) and before any
+  public traffic. **Verify** it took, and re-verify during ramp incident response:
+
+  ```bash
+  gcloud run services describe policyengine-api \
+    --project policyengine-api --region us-central1 --format yaml | grep -i minscale
+  # expect a service-level minScale annotation of 1; a minScale under
+  # spec.template (revision-level) would be the per-tag cost bomb — remove it.
+  ```
+
   Rationale: the user-facing scale-from-zero wake was measured at 282.8s — 17s under
   the 300s request timeout — so a cold start must never sit on a public request path.
 

docs/migration/history/pr4-stage2-runtime-timing.md

@@ -120,7 +120,14 @@ memory-safe.
    values untenable).
 2. Service-level `min-instances 1` (manual, runbook) + unit invariant keeping
    revision-level at 0.
-3. Consider gunicorn `--max-requests` + jitter (worker recycling) to cap the
-   slow memory growth behind the 69% knife-edge.
+3. Worker recycling (gunicorn `--max-requests`) remains a candidate mitigation
+   for the 69% memory knife-edge, but review analysis showed it must be
+   qualified on its own before adoption: the counter advances on cheap
+   requests too (at peak, a 500-request budget recycles faster than the
+   ~170s+ re-import), small jitter cannot decorrelate two workers (the
+   survivor absorbs all traffic and burns its headroom during the peer's
+   re-import), and a mid-life re-import runs without cpu-boost, CPU-throttled
+   at idle. Requires a recycling-enabled soak and sizing against the real
+   request mix (likely O(5,000+) with jitter ~50%).
 4. Re-baseline after metadata slimming lands; it dominates warm-parity and
    transfer costs.

scripts/measure_cloud_run_runtime.py

@@ -17,6 +17,7 @@
 import asyncio
 import copy
 import json
+from collections import Counter
 import random
 import sys
 import time
@@ -40,8 +41,6 @@
 def _percentile(values: list[float], percentile: float) -> float | None:
     if not values:
         return None
-    if len(values) == 1:
-        return round(values[0], 2)
     index = round((len(values) - 1) * percentile)
     return round(sorted(values)[index], 2)
 
@@ -50,19 +49,6 @@ def _utc_now_iso() -> str:
     return datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ")
 
 
-def _parse_cache_bust(raw: str) -> float:
-    if raw == "always":
-        return 1.0
-    if raw == "never":
-        return 0.0
-    share = float(raw)
-    if not 0.0 <= share <= 1.0:
-        raise argparse.ArgumentTypeError(
-            "--cache-bust must be always, never, or 0.0-1.0"
-        )
-    return share
-
-
 def _classify(status_code: int) -> str:
     if status_code == 429:
         return OUTCOME_HTTP_429
@@ -99,7 +85,7 @@ async def _client_loop(
     client: httpx.AsyncClient,
     args: argparse.Namespace,
     base_payload: dict,
-    cache_bust_share: float,
+    cache_bust: bool,
     warmup_until: float,
     deadline: float,
     records: list[dict],
@@ -111,7 +97,7 @@ async def _client_loop(
         is_calculate = rng.random() < args.calculate_share
         if is_calculate:
             payload = copy.deepcopy(base_payload)
-            if rng.random() < cache_bust_share:
+            if cache_bust:
                 payload["_loadtest"] = f"{args.cell_label}-{client_index}-{seq}"
             method, path = "POST", CALCULATE_PATH
             timeout = args.timeout_calculate
@@ -120,44 +106,32 @@ async def _client_loop(
             method, path = "GET", METADATA_PATH
             timeout = args.timeout_metadata
 
-        record = {
-            "cell": args.cell_label,
-            "client": client_index,
-            "seq": seq,
-            "endpoint": "calculate" if is_calculate else "metadata",
-            "wall_start": time.time(),
-            "warmup": time.monotonic() < warmup_until,
-        }
+        warmup = time.monotonic() < warmup_until
         started = time.perf_counter()
+        status = 0
         try:
             response = await client.request(method, path, json=payload, timeout=timeout)
-            record["latency_ms"] = round((time.perf_counter() - started) * 1000, 2)
-            record["status"] = response.status_code
-            record["bytes"] = len(response.content)
-            record["outcome"] = _classify(response.status_code)
+            status = response.status_code
+            outcome = _classify(status)
         except httpx.TimeoutException:
-            record["latency_ms"] = round((time.perf_counter() - started) * 1000, 2)
-            record["status"] = 0
-            record["bytes"] = 0
-            record["outcome"] = OUTCOME_TIMEOUT
-        except httpx.HTTPError as error:
-            record["latency_ms"] = round((time.perf_counter() - started) * 1000, 2)
-            record["status"] = 0
-            record["bytes"] = 0
-            record["outcome"] = OUTCOME_CONNECT_ERROR
-            record["error"] = f"{type(error).__name__}: {error}"
-        records.append(record)
+            outcome = OUTCOME_TIMEOUT
+        except httpx.HTTPError:
+            outcome = OUTCOME_CONNECT_ERROR
+        records.append(
+            {
+                "endpoint": "calculate" if is_calculate else "metadata",
+                "warmup": warmup,
+                "latency_ms": round((time.perf_counter() - started) * 1000, 2),
+                "status": status,
+                "outcome": outcome,
+            }
+        )
 
 
 def _summarize_endpoint(records: list[dict], duration_seconds: float) -> dict:
     latencies = [r["latency_ms"] for r in records if r["outcome"] == OUTCOME_OK]
-    outcome_counts: dict[str, int] = {}
-    status_counts: dict[str, int] = {}
-    for record in records:
-        outcome_counts[record["outcome"]] = outcome_counts.get(record["outcome"], 0) + 1
-        status_counts[str(record["status"])] = (
-            status_counts.get(str(record["status"]), 0) + 1
-        )
+    outcome_counts = Counter(r["outcome"] for r in records)
+    status_counts = Counter(str(r["status"]) for r in records)
     return {
         "request_count": len(records),
         "achieved_rps": round(len(records) / duration_seconds, 3),
@@ -166,15 +140,15 @@ def _summarize_endpoint(records: list[dict], duration_seconds: float) -> dict:
         "p95_latency_ms": _percentile(latencies, 0.95),
         "p99_latency_ms": _percentile(latencies, 0.99),
         "max_latency_ms": max(latencies) if latencies else None,
-        "outcome_counts": outcome_counts,
-        "status_counts": status_counts,
+        "outcome_counts": dict(outcome_counts),
+        "status_counts": dict(status_counts),
     }
 
 
 async def _run(args: argparse.Namespace) -> dict:
     base_payload = json.loads(Path(args.calculate_payload).read_text())
     base_payload["household"]["axes"][0][0]["count"] = args.axes_count
-    cache_bust_share = _parse_cache_bust(args.cache_bust)
+    cache_bust = args.cache_bust == "always"
 
     limits = httpx.Limits(max_connections=args.clients + 2)
     async with httpx.AsyncClient(
@@ -200,7 +174,7 @@ async def _run(args: argparse.Namespace) -> dict:
                     client=client,
                     args=args,
                     base_payload=base_payload,
-                    cache_bust_share=cache_bust_share,
+                    cache_bust=cache_bust,
                     warmup_until=warmup_until,
                     deadline=deadline,
                     records=records,
@@ -248,8 +222,10 @@ def main() -> int:
     parser.add_argument("--axes-count", type=int, default=101)
     parser.add_argument(
         "--cache-bust",
+        choices=("always", "never"),
         default="always",
-        help="always | never | fraction 0.0-1.0 of calculate requests busted",
+        help="always = every calculate does real engine work (capacity cells); "
+        "never = identical payloads measure the Redis cache path",
     )
     parser.add_argument("--seed", type=int, default=42)
     parser.add_argument(
@@ -264,7 +240,6 @@ def main() -> int:
         help="seconds to wait for /readiness-check (cold imports take ~3 min)",
     )
     args = parser.parse_args()
-    _parse_cache_bust(args.cache_bust)
 
     summary = asyncio.run(_run(args))
     json.dump(summary, sys.stdout, indent=2)

tests/unit/test_cloud_run_deploy_scripts.py

@@ -369,16 +369,11 @@ def test_deploy_cloud_run_candidate_pins_runtime_shape():
     )
 
     assert result.returncode == 0, result.stderr
-    # Stage 2 qualification values, pinned explicitly on every deploy so a
-    # polluted service template can never leak into a candidate (gcloud
-    # inherits unspecified fields; `--concurrency default` is the platform
-    # default of 640, not the historical 80).
-    assert "--concurrency 4" in result.stdout
-    assert "WEB_CONCURRENCY=2" in result.stdout
-    # Warm capacity is a service-level setting made outside CI: revision-level
-    # min-instances above zero would keep a warm 16Gi instance per
-    # accumulated revision tag.
-    assert "--min-instances 0" in result.stdout
+    # Stage 2-qualified values, pinned on every deploy — rationale in
+    # docs/migration/cloud-run-operations.md ("Runtime shape and scaling").
+    assert "--concurrency 4 " in result.stdout
+    assert "WEB_CONCURRENCY=2 " in result.stdout
+    assert "--min-instances 0 " in result.stdout
 
 
 def test_push_workflow_pins_cloud_run_scaling_per_job():