Make quality snapshots enforce performance and coverage regressions

[karthiknadig]

Summary

PET's quality snapshot workflows currently report regressions but do not reliably enforce them, and parts of the E2E client can distort measurements.

A longitudinal audit of 11 successful main-branch baselines (July 24-August 6, 2026) found stable/rising coverage, but also showed that the workflows can stay green while meaningful tail latency or coverage drift is present.

Concrete problems

• .github/workflows/perf-tests.yml runs the benchmark with continue-on-error: true.
• Missing benchmark output is replaced with a successful zero-valued metrics.json.
• P95 is collected and displayed, but regression indicators and decisions compare only P50.
• Environment/manager inventory changes are displayed but not validated, so unlike workloads can be compared.
• Coverage decreases only change the PR comment; they do not fail the quality gate.
• The E2E PetClient pipes child stderr without draining it and recreates a BufReader around stdout for every request. Pipe backpressure or discarded read-ahead can invalidate latency measurements.

This means issue #313's regression-detection acceptance criterion is only partially satisfied: snapshots are generated, but quality regressions are not consistently detected or gated.

Tasks

• Make malformed/missing benchmark metrics fail instead of producing zero-shaped success.
• Make the E2E client continuously drain stderr and preserve framed stdout buffering for the process lifetime.
• Compare and gate P50, P95, and time-to-first-environment using explicit per-platform tolerances.
• Reject or clearly invalidate comparisons when discovered environment/manager counts differ from baseline.
• Enforce a documented coverage budget; new code must remain covered and unexplained decreases must fail.
• Extract report/comparison logic into testable code rather than duplicating shell arithmetic across platforms.
• Preserve full artifacts and summaries for longitudinal analysis.

Acceptance criteria

• A benchmark process/test failure cannot produce a successful zero-valued snapshot.
• P50, P95, and time-to-first-environment regressions are evaluated against baseline.
• Workload inventory mismatches invalidate the comparison.
• Coverage regressions fail unless an explicit reviewed exception is used.
• E2E JSON-RPC stdout/stderr handling cannot block or discard buffered protocol data.
• Comparison/gating logic has unit tests for improvement, noise, regression, missing data, and inventory mismatch.
• Existing per-platform snapshot comments and artifacts remain available.

Related

• Add end-to-end performance tests for JSONRPC server across platforms #313
• Separate macOS tail-latency investigation filed from the same audit.

[karthiknadig]

Additional concrete finding from the longitudinal audit:

• PR workflows download baseline artifacts by branch: main, so a long-running PR can be compared with a newer unrelated main commit rather than github.event.pull_request.base.sha.
• This already produced misleading coverage drift (for example, a documentation-only branch inherited a Windows coverage decrease after another PR changed the latest baseline).

The fix must pin baseline selection to the PR base SHA (or compute the base snapshot in the same run) and fail clearly if the exact baseline is unavailable. It must not silently compare against a moving branch tip.