From cf1b3dc58760e1414fda28669d9b4c15b9b67d2c Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Fri, 23 Oct 2026 06:06:14 -0500 Subject: [PATCH] fix(api): reserve capacity for review page resources Co-Authored-By: Claude Opus 5.5 --- backend/server/api/routes/result.py | 14 +++++--- backend/server/api/tests/test_review_view.py | 36 +++++++++++++++++--- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/backend/server/api/routes/result.py b/backend/server/api/routes/result.py index a4995a33..c8cd4d57 100644 --- a/backend/server/api/routes/result.py +++ b/backend/server/api/routes/result.py @@ -252,9 +252,13 @@ def _charge_page_resource(deps, holder: str, signed_capture: str | None) -> None gallery over 120 photos exhausted it while images loaded, and the rest of the page and the reviewer's ordinary result reads got 429. The page shows one photo per keyframe, and keyframes are capture files, so one page load - needs at most HSS_INGEST_MAX_FILES photo requests plus a few raw-file, - module and preview links. Twice HSS_INGEST_MAX_FILES holds a full page with - room for a reload or a second tab; no traffic measurement backs the factor. + needs at most HSS_INGEST_MAX_FILES photo requests. Reserve a floor for + ten fixed requests: the module, preview fetch, four verdict JSONs, criteria + report, two result JSONs, and preview download. Without this floor, a file + limit of one refuses the third resource on an otherwise valid page. + + The retained 2x photo factor has no traffic evidence behind it. This limiter + counts per process: N containers allow N times the budget, not a shared cap. A signed request's bucket is keyed by reviewer and the capture its token names, so pages for different captures opened together do not share one. @@ -263,7 +267,9 @@ def _charge_page_resource(deps, holder: str, signed_capture: str | None) -> None on it would hand out a fresh bucket per made-up id. """ - capacity = 2 * int(deps.settings.HSS_INGEST_MAX_FILES) + max_photos = int(deps.settings.HSS_INGEST_MAX_FILES) + fixed_requests = 1 + 1 + 4 + 1 + 2 + 1 + capacity = max(2 * max_photos, max_photos + fixed_requests) key = holder if signed_capture is None else f"{holder}/{signed_capture}" deps.charge(key, "review-page", capacity) diff --git a/backend/server/api/tests/test_review_view.py b/backend/server/api/tests/test_review_view.py index 307d69d8..10afa635 100644 --- a/backend/server/api/tests/test_review_view.py +++ b/backend/server/api/tests/test_review_view.py @@ -929,8 +929,8 @@ def test_page_resources_are_rate_limited_including_the_module(client: TestClient """The page bucket is a real limit, and the vendored module spends it too. The module route charged nothing, so a signed import link could be fetched - without bound. With HSS_INGEST_MAX_FILES=3 the page bucket holds six - requests; the seventh is refused, and the reviewer's ``get`` reads are not. + without bound. With HSS_INGEST_MAX_FILES=3 the page bucket holds thirteen + requests; the fourteenth is refused, and the reviewer's ``get`` reads are not. """ capture_id = _create(client, "packet-review-0020") @@ -941,6 +941,34 @@ def test_page_resources_are_rate_limited_including_the_module(client: TestClient module = re.search(r'import \* as THREE from "(/v1/review-static/three\.module\.js\?rtok=[^"]+)"', served.text) assert module, "the served import carries no signed review token" bare = TestClient(client.app) - statuses = [bare.get(module.group(1)).status_code for _ in range(7)] - assert statuses == [200] * 6 + [429] + statuses = [bare.get(module.group(1)).status_code for _ in range(14)] + assert statuses == [200] * 13 + [429] assert client.get(f"/v1/captures/{capture_id}/result", headers=REVIEWER_HEADERS).status_code == 200 + + +def test_one_photo_page_loads_all_resources_with_one_file_limit(client: TestClient): + """The minimum ingest limit must still allow a whole review page.""" + + capture_id = _create(client, "packet-review-capacity") + client.app_state.settings.HSS_INGEST_MAX_FILES = 1 + _seed( + client, + capture_id, + _render_review_html(capture_id, signed_photo=None, signed_link=None), + scene={"keyframes": [{"id": "kf_1", "image": "stills/kf_01.jpg"}]}, + still=b"\xff\xd8fakejpeg", + preview=b"preview", + ) + names = [f"verdicts/verdict.{profile}.json" for profile in ("A", "B", "C", "S")] + names += ["verdicts/criteria_report.json", "result/result.json", "result/result.internal.json"] + for name in names: + client.app_state.storage.put_bytes(f"captures/{capture_id}/runs/{RUN_ID}/{name}", b"{}", "application/json") + _freeze_limiter(client) + served = client.get(f"/v1/captures/{capture_id}/review", headers=REVIEWER_HEADERS) + assert served.status_code == 200 + urls = _page_subresources(served.text, capture_id) + assert len(urls) == 11 # one photo, module, preview fetch, and eight raw-file links + bare = TestClient(client.app) + for url in urls: + response = bare.get(url) + assert response.status_code == 200, (url, response.status_code, response.text[:200])