Skip to content

fix(smoke): make the screenshot-regression suite deterministic (pixel compare + blank/partial retry + Termina seam fixes)#1451

Merged
Aaronontheweb merged 18 commits into
netclaw-dev:devfrom
Aaronontheweb:fix/racy-test-determinism
Jun 23, 2026
Merged

fix(smoke): make the screenshot-regression suite deterministic (pixel compare + blank/partial retry + Termina seam fixes)#1451
Aaronontheweb merged 18 commits into
netclaw-dev:devfrom
Aaronontheweb:fix/racy-test-determinism

Conversation

@Aaronontheweb

@Aaronontheweb Aaronontheweb commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

Makes the VHS screenshot-regression smoke suite deterministic. What started as
one flaky config-search tape turned out to be several independent sources of
non-determinism — some in the test harness, some genuine timing seams in the
Termina TUI framework. This PR fixes them at the source and adds a defense-in-depth
retry so a transient capture can never produce a false fail.

Validated by 10 consecutive green "Screenshot Regression (Linux)" runs on the
final HEAD (full reruns, not single-job), with config-search-selection — the last
holdout — passing at AE=0 every time.

Test-harness changes (scripts/smoke/run-smoke.sh, .github/workflows/smoke.yml)

  • Pixel comparison instead of byte-for-byte. VHS's PNG zlib encoder is not
    deterministic across processes (same pixels, different byte stream), so cmp -s
    false-failed. Now compares with ImageMagick compare -metric AE; ImageMagick is
    installed in the screenshot CI job.
  • Cursor-cell tolerance. Set CursorBlink false freezes the shell-prompt cursor
    but not its on/off state; the block is ~1 character cell (~493 px). The AE
    tolerance clears a single cursor cell while still failing on real content changes.
  • Blank / partial-frame retry. A frame that is fully blank (unique-color count
    < 16) or differs from baseline by more than the tolerance is treated as a
    transient capture and the tape is re-run (bounded). A real regression reproduces
    every attempt and still fails at compare time — only latency differs.
  • bash 3.2 compatibility. macOS runners ship bash 3.2 (no associative arrays);
    the tape→frames map is a case function instead of declare -A, which was
    aborting the Native Smoke (macOS) job under set -u.

Tape changes (tests/smoke/tapes/screenshots/*.tape)

  • Replaced Sleep-based timing guards with deterministic Wait+Screen anchors
    across config-search, mcp-permissions, provider-manager, wizard-screens,
    and help.
  • Anti-blank re-asserts immediately before vulnerable screenshots (e.g. the provider
    picker's last list row; the Ollama row before the provider-manager capture).
  • Refreshed two stale baselines (help, config-search-brave-entry).

Root-cause fixes in Termina (dependency bumps)

Two of the failures were genuine framework timing seams, fixed upstream and pulled
in here. The harness retries are now pure defense-in-depth:

  • 0.14.0-beta.2termina#311:
    a no-op resize triggered a full-screen clear + repaint (\e[2J), which VHS could
    sample mid-write → blank/partial frames.
  • 0.14.0-beta3termina#314:
    deferred navigation rendered the old page one extra time on the navigation seam,
    so a screenshot taken right after Enter could capture the page you just left
    (the config-search-selection wrong-page capture).

Validation

  • dotnet slopwatch analyze — no new violations (no production code changed).
  • Screenshot Regression (Linux): 10/10 consecutive green on the beta3 HEAD.
  • Native Smoke (Linux + macOS): green (bash 3.2 fix confirmed).

…rs in config-search screenshot tape

The three config-search screenshot frames (selection, brave-entry, saved)
were failing consistently across unrelated Dependabot PRs because the tape
used Sleep 1s before each Screenshot instead of deterministic screen anchors.

Root cause (per Akka.NET async state machine and .NET concurrency analysis):

Frames 1 and 2: Sleep 1s was dead weight. SelectBackendForEditing and the
ProviderSelection navigation are both fully synchronous — they complete on
the Termina event loop thread before the next render. Wait+Screen on the
heading text is the only gate needed.

Frame 3: Sleep 1s was insufficient. SaveWithoutProbeOverride fires four
synchronous ReactiveProperty cascade invalidations (two Revalidate calls, an
ActiveDialog reset, and the CurrentScreen=Saved assignment) plus a
ReloadPersistedDraft disk read. Each invalidation re-evaluates
DynamicLayoutNode._factory(), queuing an additional RedrawRequested event.
A single Wait+Screen on 'validated and saved' matches after the first stable
render but before all queued redraws have settled. Adding a second anchor on
'[Enter] Settings Areas' — text emitted by BuildKeyBindings only when
CurrentScreen==Saved and ActiveDialog==None (SearchConfigEditorPage.cs:212)
— guarantees both reactive properties have committed to their final state in
a single stable rendered frame before the screenshot fires.
…condary anchors to mcp-permissions

All screenshot tapes used Sleep 1s before and after Screenshot commands
as a timing guard. For the screens affected, Wait+Screen is sufficient:

- help.tape: static CLI output; nothing renders after the text appears
- provider-manager.tape: synchronous list render on navigation
- wizard-screens.tape (frames 1 and 2): synchronous wizard step transitions

The Sleep 1s after Ctrl+Q in wizard-screens.tape is intentionally kept —
that is the alt-screen restore guard (CSI ?1049l), not a screenshot guard.

mcp-permissions.tape gets secondary Wait+Screen anchors instead of Sleep:
- Frame 1: Wait+Screen@5s /3 tools/ confirms tool count has loaded
- Frame 2: Wait+Screen@5s /Server default/ confirms header rows are present
  alongside the tool rows (direct regression guard for netclaw-dev#1424)

Updated stale header comments in provider-manager.tape and wizard-screens.tape
that still described the removed Sleep 1s approach.
…onfig-search baselines

Two separate fixes:

help.tape and provider-manager.tape: restored Sleep 1s before the Screenshot
command. These tapes fire their screenshot right at the start of the tape
(not mid-flow), and VHS may not have flushed all terminal output into its
buffer by the time Wait+Screen matches the first visible line. The Sleep is
a genuine render-settle guard, not timing slop. Removed the post-screenshot
Sleep (that was pure pacing and is not needed).

config-search baselines (approved.png x3): the three config-search screenshot
frames had stale baselines that predated a UI change — they were failing
consistently on every recent Dependabot PR regardless of what code was touched.
Promoted the actual screenshots from CI run 27842198884 (captured with the new
deterministic Wait+Screen anchors from the previous commits) as the new
approved baselines.
@Aaronontheweb Aaronontheweb added the tests All issues related to testing, quality assurance, and smoke testing. label Jun 19, 2026
…cp-permissions settle

config-search.tape: add secondary Wait+Screen on DuckDuckGo description text
to guard against leaked Down key from Settings navigation moving the cursor to
Brave before the screenshot fires.

mcp-permissions.tape: add Sleep 1s settle guard between server-list screenshot
and Enter navigation (daemon can push a tool re-index update in this window,
causing Enter to hit the TUI mid-transition and produce a blank tool-grid
screenshot); increase tool-grid anchor timeouts to 15s/10s.
…-byte cmp

VHS's PNG zlib encoder produces different byte streams for visually identical
renders across process invocations (filter-heuristic jitter). cmp -s false-
fails on such runs even though zero pixels differ.

Switch compare_shot_frame to use ImageMagick's 'compare -metric AE', which
counts differing pixels and exits 0 when none differ. This tolerates encoder
jitter while still catching real visual regressions (cursor on wrong row,
missing UI element, etc.). Falls back to cmp -s when ImageMagick is absent.

ImageMagick is already present in CI (used for the existing diff PNG path).
The pixel-comparison fix in run-smoke.sh uses 'compare -metric AE' but
ImageMagick was not present in the Screenshot Regression (Linux) job,
causing it to fall back to cmp -s and false-fail on PNG encoder jitter.
…500)

Set CursorBlink false freezes the terminal cursor but not its on/off state.
The cursor block (~160px) can be visible or hidden non-deterministically
between runs even with blinking disabled. A hard AE=0 requirement false-fails
on this noise.

Allow up to 500 differing pixels — covers the cursor block with margin, while
still catching real regressions: a misplaced cursor row differs by thousands
of pixels, a blank screen by millions.
…-refresh artifact

Root cause: under VHS, Termina emits a full-screen clear (\e[2J) + repaint as
one atomic write when a startup ResizeEvent fires (ConsoleInputSource polls
Console.WindowWidth every 10ms; the PTY size jitters as it initialises). VHS can
sample the screenshot between the clear and the repaint half of that same write,
capturing a blank frame (AE~68000, 1 unique color). The write is atomic from the
app side — real users never see it; only VHS's mid-write PTY sampling does.
Amplified by the Termina 0.12.1->0.14.0-beta.1 bump, which routes spinners
through RenderFrameProvider and raises render cadence.

Deterministic-in-outcome fix (test harness, production code is correct):
- run-smoke.sh: detect a transient blank capture via unique-color count
  (identify %k < 16; blank=1 color, sparsest real frame=7627) and re-run that
  tape up to SHOT_BLANK_RETRIES times. A blank can no longer cause a false
  fail; it was never a false pass (blank != baseline).
- provider-manager.tape: re-assert the list anchor immediately before the
  screenshot (was Sleep-then-capture, which reopened the blank window).

Also widen the cursor-cell tolerance 500->1000 (AE): the shell-prompt cursor
block measures ~493px (one cell); 500 left only 7px margin. 1000 (~2 cells)
clears a single cursor cell while still failing on real text changes.
macOS runners ship bash 3.2 (no associative arrays). The SHOT_TAPE_FRAMES
`declare -A` parsed its [help]=... entries as indexed-array assignments and
aborted under `set -u` with 'line 94: help: unbound variable', failing the
Native Smoke (macOS) job (which loads run-smoke.sh for the light/full modes,
not just screenshots). Replace the associative array with a portable
shot_tape_frames() case function. No behavior change on Linux.
…inism

# Conflicts:
#	tests/smoke/screenshots/config-search-brave-entry.approved.png
The Termina full-refresh artifact also manifests as a PARTIAL frame: VHS
samples mid-repaint so only the top rows land (seen on mcp-permissions-tool-grid
— header + Audience row present, but Server enabled/default and the add/echo/
record-tasks rows missing; AE=19047 vs baseline, yet 4467 colors so the
color-count blank detector misses it).

Generalize the retry trigger from frame_is_blank to frame_needs_retry: blank
(unique-color<16) OR differs from baseline by > SHOT_AE_TOLERANCE. A real
regression also trips the AE branch but reproduces every attempt, so it still
fails at compare time — only latency differs. Tolerance is now a shared global
used by both the retry trigger and the compare gate.
Picks up the upstream fix for Aaronontheweb/termina#311: skip the full-screen
ForceFullRefresh on a no-op resize. This eliminates the blank/partial screenshot
frames at the source (a startup ResizeEvent no longer triggers a [2J clear +
repaint that VHS could sample mid-write). The harness blank/partial retry stays
as defense-in-depth.
@Aaronontheweb Aaronontheweb changed the title fix(smoke): replace Sleep guards with deterministic Wait+Screen anchors in config-search screenshot tape fix(smoke): make the screenshot-regression suite deterministic (pixel compare + blank/partial retry + Termina seam fixes) Jun 23, 2026
@Aaronontheweb
Aaronontheweb enabled auto-merge (squash) June 23, 2026 01:05
@Aaronontheweb
Aaronontheweb merged commit 8c4234c into netclaw-dev:dev Jun 23, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests All issues related to testing, quality assurance, and smoke testing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant