From ce50f641cc8776c6405ed7da59ac4e8eb0bac779 Mon Sep 17 00:00:00 2001 From: autonomic-bot Date: Thu, 11 Jun 2026 01:31:03 +0000 Subject: [PATCH] =?UTF-8?q?feat(shot):=20harness=20default=20capture=20fix?= =?UTF-8?q?=20=E2=80=94=20bounded=20networkidle=20settle=20after=20domcont?= =?UTF-8?q?entloaded=20+=20blank-frame=20retry=20(=E2=89=A460s=20wait=20bu?= =?UTF-8?q?dget,=20R7=20best-effort=20preserved);=206=20unit=20tests;=20li?= =?UTF-8?q?nt=20PASS,=20205=20unit=20tests=20pass=20via=20cc-ci-run?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- runner/harness/screenshot.py | 56 ++++++++++++++++++++++- tests/unit/test_screenshot.py | 84 +++++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 2 deletions(-) diff --git a/runner/harness/screenshot.py b/runner/harness/screenshot.py index 66c178a..7527764 100644 --- a/runner/harness/screenshot.py +++ b/runner/harness/screenshot.py @@ -18,6 +18,7 @@ missing, app slow, navigation error) is swallowed and returns None so the run/ve from __future__ import annotations +import contextlib import os from . import browser as harness_browser @@ -28,6 +29,55 @@ VIEWPORT = {"width": 1280, "height": 800} # Hard cap so a wedged app can never hang the run on the screenshot step (R7 / Phase-1 timeouts). NAV_DEADLINE_S = 45 +# ---- post-navigation settle (phase-shot fix, 2026-06-11) ---- +# SPAs (immich, n8n, cryptpad, the keycloak admin console, lasuite-*, mumble-web, mattermost) fire +# `domcontentloaded` on their empty HTML shell and only paint after the JS bundle loads — snapping +# immediately produced solid blank frames (byte-stable 4801-2 B) or loading spinners. After nav, +# wait for network-idle up to SETTLE_TIMEOUT_MS (apps that never go idle — continuous polling — +# simply spend the cap; bounded, never raises), then RENDER_GRACE_MS for the final paint. +SETTLE_TIMEOUT_MS = 10_000 +RENDER_GRACE_MS = 500 +# A 1280x800 PNG below this is near-certainly a solid frame or a bare loading spinner (phase-shot +# audit: blank frames were 4801-2 B across three different apps, lone spinners 5.9-8.8 KB; the +# smallest real page was 12950 B). One bounded retry with an extra settle, then keep what we get — +# an honest late frame beats none, and the retry only ever replaces a tiny frame with a later one. +BLANK_SIZE_BYTES = 10_000 +BLANK_RETRY_SETTLE_MS = 4_000 +# Wait-budget arithmetic (plan-phase-shot §3 P3: step worst case ≤ ~60s): NAV_DEADLINE_S (45s, +# spent only while the app isn't serving yet) + SETTLE_TIMEOUT_MS + RENDER_GRACE_MS + +# BLANK_RETRY_SETTLE_MS + RENDER_GRACE_MS = 60s of bounded waiting; tested in unit tests. + + +def _settle(page, idle_timeout_ms: int) -> None: + """Best-effort bounded settle: network-idle up to the cap, then a short render grace. + Never raises (R7) — a timeout just means the page kept polling; we snap what's painted.""" + # cosmetic path (R7): a timeout on a never-idle app is expected — the cap IS the wait + with contextlib.suppress(Exception): + page.wait_for_load_state("networkidle", timeout=idle_timeout_ms) + with contextlib.suppress(Exception): + page.wait_for_timeout(RENDER_GRACE_MS) + + +def _snap_with_blank_retry(page, out_path: str) -> None: + """Screenshot the page; if the PNG is blank/spinner-sized, retry ONCE after a longer settle. + The retry overwrites the tiny frame with a strictly-later one (same page, more paint time).""" + page.screenshot(path=out_path, full_page=False) + try: + first = os.path.getsize(out_path) + except OSError: + return + if first >= BLANK_SIZE_BYTES: + return + print( + f" screenshot: frame looks blank/loading ({first} B < {BLANK_SIZE_BYTES} B) — " + "one retry after a longer settle", + flush=True, + ) + _settle(page, BLANK_RETRY_SETTLE_MS) + page.screenshot(path=out_path, full_page=False) + with contextlib.suppress(OSError): + print(f" screenshot: retry frame {os.path.getsize(out_path)} B", flush=True) + def screenshot_path(run_artifact_dir: str) -> str: """Canonical on-disk path for a run's app screenshot (pure).""" @@ -79,7 +129,7 @@ def capture(domain: str, out_path: str, *, recipe_meta: dict | None = None) -> s # the uniform ctx convention (rcust P3). hook(page, meta_mod.hook_ctx(domain, recipe_meta)) if not os.path.exists(out_path): - page.screenshot(path=out_path, full_page=False) + _snap_with_blank_retry(page, out_path) else: # Default: landing page. Accept any rendered status (200 or an auth redirect to a # login form) — both are credential-free and representative of "the app is up". @@ -90,7 +140,9 @@ def capture(domain: str, out_path: str, *, recipe_meta: dict | None = None) -> s deadline_seconds=NAV_DEADLINE_S, wait_until="domcontentloaded", ) - page.screenshot(path=out_path, full_page=False) + # SPA paint race fix (phase-shot): settle before snapping, retry a blank frame. + _settle(page, SETTLE_TIMEOUT_MS) + _snap_with_blank_retry(page, out_path) finally: browser.close() if os.path.exists(out_path) and os.path.getsize(out_path) > 0: diff --git a/tests/unit/test_screenshot.py b/tests/unit/test_screenshot.py index a5cc277..43968bc 100644 --- a/tests/unit/test_screenshot.py +++ b/tests/unit/test_screenshot.py @@ -32,6 +32,90 @@ def test_hook_returned_when_callable(): assert S._load_screenshot_hook({"SCREENSHOT": hook}) is hook +class _FakePage: + """Minimal Playwright-page stand-in for the settle/blank-retry helpers (no browser needed).""" + + def __init__(self, shot_sizes, idle_raises=False): + self._shot_sizes = list(shot_sizes) # bytes written per successive screenshot() call + self._idle_raises = idle_raises + self.idle_waits = [] # (state, timeout) per wait_for_load_state call + self.timeout_waits = [] # ms per wait_for_timeout call + self.shots = 0 + + def wait_for_load_state(self, state, timeout=None): + self.idle_waits.append((state, timeout)) + if self._idle_raises: + raise TimeoutError(f"page kept polling past {timeout}ms") + + def wait_for_timeout(self, ms): + self.timeout_waits.append(ms) + + def screenshot(self, path, full_page=False): + self.shots += 1 + with open(path, "wb") as f: + f.write(b"\x89PNG" + b"\0" * (self._shot_sizes.pop(0) - 4)) + + +def test_settle_swallows_never_idle_pages(): + """R7: an app that never reaches network-idle (continuous polling) must not raise — the + timeout cap IS the wait.""" + page = _FakePage([], idle_raises=True) + S._settle(page, 1234) # must not raise + assert page.idle_waits == [("networkidle", 1234)] + assert page.timeout_waits == [S.RENDER_GRACE_MS] + + +def test_snap_retries_blank_frame(tmp_path): + """A blank-sized first frame (audit fingerprint: 4801 B) triggers exactly one retry with a + longer settle, overwriting the tiny frame with the later (painted) one.""" + out = str(tmp_path / "shot.png") + page = _FakePage([4801, 30256]) + S._snap_with_blank_retry(page, out) + assert page.shots == 2 + assert page.idle_waits == [("networkidle", S.BLANK_RETRY_SETTLE_MS)] + assert os.path.getsize(out) == 30256 + + +def test_snap_no_retry_for_real_frame(tmp_path): + """A real-sized first frame is kept as-is — no second screenshot, no extra waiting.""" + out = str(tmp_path / "shot.png") + page = _FakePage([35707]) + S._snap_with_blank_retry(page, out) + assert page.shots == 1 + assert page.idle_waits == [] + assert os.path.getsize(out) == 35707 + + +def test_snap_retry_keeps_late_frame_even_if_still_blank(tmp_path): + """If the retry frame is still tiny we keep it (honest best-effort) — exactly one retry, + never a loop.""" + out = str(tmp_path / "shot.png") + page = _FakePage([4801, 4801]) + S._snap_with_blank_retry(page, out) + assert page.shots == 2 + assert os.path.getsize(out) == 4801 + + +def test_blank_threshold_brackets_observed_sizes(): + """Threshold sits between the audited defect sizes (blank 4801-2 B, lone spinners up to + 8764 B) and the smallest real page (custom-html-tiny, 12950 B).""" + for defect in (4801, 4802, 5895, 6022, 7913, 8764): + assert defect < S.BLANK_SIZE_BYTES + assert S.BLANK_SIZE_BYTES < 12950 + + +def test_wait_budget_within_step_cap(): + """plan-phase-shot §3 P3: the screenshot step's bounded waiting must stay ≤ ~60s worst case.""" + total_ms = ( + S.NAV_DEADLINE_S * 1000 + + S.SETTLE_TIMEOUT_MS + + S.RENDER_GRACE_MS + + S.BLANK_RETRY_SETTLE_MS + + S.RENDER_GRACE_MS + ) + assert total_ms <= 60_000, f"screenshot wait budget {total_ms}ms exceeds the ~60s step cap" + + def test_screenshot_reachable_through_real_load_path(tmp_path): """R2 proof (rcust P1): a recipe SCREENSHOT hook declared in recipe_meta.py arrives at screenshot._load_screenshot_hook through the REAL orchestrator load path (meta.load — the