feat: implement issue #1698 — qa-lead over-escalates test flakiness at production tier — the one held-out case parity did NOT fix (HIGH/escalate where MEDIUM/no is correct)

.github/workflows/lint.yml

@@ -181,6 +181,7 @@
             tests/dev-lead/unit/test_fewshot_assemble.bats \
             tests/test_qa_lead_test_surface.bats \
             tests/test_qa_lead_advisory_gate.bats \
+            tests/test_qa_lead_holdout_calibration.bats \
             tests/test_advisory_offline_mode.bats \
             tests/agent_ingress_reference.bats \
             tests/agent_ingress_if_filter.bats
@@ -251,7 +252,7 @@
     timeout-minutes: 5
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
         # NOTE: persist-credentials is intentionally left at the default (true) —
         # vci_resolve_reusable needs `git fetch origin` to authenticate against this
         # private repo when resolving same-repo channel tags. Setting
         # persist-credentials: false would silently push every such caller into the

evals/qa-lead/holdout/cases.jsonl

@@ -4,3 +4,6 @@
 {"id": "qa-lead-hold-flaky-network-e2e", "description": "E2E depends on a live third-party endpoint, causing nondeterminism. Flakiness/isolation issue.", "tags": ["flakiness", "isolation"], "input": "PR's new E2E hits a real external payments sandbox over the network in CI; it fails ~10% of runs when the sandbox is slow.", "expected": {"escalate": false, "risk": "MEDIUM", "recommend": "stub/mock the external dependency at the boundary (or contract-test it) so CI is deterministic"}}
 {"id": "qa-lead-hold-payment-happy-only", "description": "Payment capture path tested only for success; missing failure/idempotency/edge. High risk on money path.", "tags": ["payments", "api"], "input": "PR adds card-capture handling; tests cover a successful capture only — no declined-card, no duplicate-submit/idempotency-key, no partial-refund path.", "expected": {"escalate": true, "risk": "HIGH", "recommend": "add declined, duplicate-submit (idempotency), and refund cases before merge — money paths are high-impact"}}
 {"id": "qa-lead-hold-coverage-gaming", "description": "Tests execute code but assert nothing meaningful, inflating coverage. Reward-hack-resistant: high coverage != quality.", "tags": ["security-anti-pattern", "coverage"], "input": "PR raises line coverage to 95% by adding tests that call functions and assert only that no exception is thrown (no output/behavior assertions).", "expected": {"escalate": true, "risk": "MEDIUM", "recommend": "coverage is gamed; require behavioral assertions on outputs/side-effects, not exception-free execution"}}
+{"id": "qa-lead-hold-flaky-ui-timing", "description": "UI E2E is nondeterministic because it races a real fade-in animation via a fixed timing wait, not because of any production defect. A test-harness flakiness/isolation symptom with no production impact — remediate as tech debt, do NOT escalate. Grounds: 'flakiness is critical tech debt' AND 'risk vs value' (a harness timing symptom is not a merge-blocking production risk).", "tags": ["flakiness", "isolation"], "input": "PR adds a UI E2E that sleeps a fixed 500ms for a fade-in animation before asserting a banner is visible; it fails ~8% of runs on slower CI runners when the animation has not finished. The component under test is unchanged and behaves correctly in production.", "expected": {"escalate": false, "risk": "MEDIUM", "recommend": "replace the fixed animation-timing sleep with a deterministic web-first assertion on the element's visible/stable state so CI is deterministic; no production change is needed"}}
+{"id": "qa-lead-hold-flaky-masks-real-race", "description": "The test is flaky because the PRODUCTION code does an unsynchronized read-modify-write on shared state; the nondeterminism originates in the product, not the harness, and papering it over with retries hides a real concurrency defect. High-impact production risk — escalate. Grounds: 'depth scales with impact'; do not mask a real race with retries.", "tags": ["flakiness", "concurrency"], "input": "PR makes an intermittently-failing test pass by wrapping it in a retry-3-times loop. The test drives two concurrent requests that increment a shared in-memory counter with no locking; under load the production handler loses updates — exactly what the test was catching intermittently — and the PR adds no synchronization to the handler.", "expected": {"escalate": true, "risk": "HIGH", "recommend": "fix the underlying production race (synchronize/lock the shared state or make the update atomic); do not mask the concurrency defect with test retries"}}
+{"id": "qa-lead-hold-slow-deterministic-suite", "description": "A reliable, fully deterministic suite that is merely slow — a CI-speed/toil concern, not a correctness or flakiness one. The QA Lead must NOT escalate a slow-but-green suite. Grounds: 'risk vs value'; negative control against escalating any CI-signal problem.", "tags": ["negative", "ci-speed"], "input": "PR adds an integration test that reliably passes on every run but takes ~90s because it spins up a real container fixture; it slows the CI job noticeably but is fully deterministic with no intermittent failures.", "expected": {"escalate": false, "risk": "LOW", "recommend": "keep the deterministic coverage; optionally speed it up (share/parallelize the fixture or push some assertions to a lower test level), but it is not a correctness risk and needs no escalation"}}

prompts/qa-lead/advisory.md

@@ -89,6 +89,50 @@ Everything else below — the risk assessment and the sentinel-wrapped output sh
    absent NFR/perf, fixture/isolation problems, flakiness); and whether anything
    is severe enough to **escalate** (you cannot block, only say so).
 
+   ### Calibrating the risk tier (read before you tier)
+
+   The tier measures the **blast radius of the change on production and users if
+   the test gap ships** — *not* how annoying the test symptom is. Rank the
+   production consequence, then tier; do not rank the inconvenience of the failing
+   test. This is calibration, not caution: escalate the changes that genuinely
+   need it and let the rest through.
+
+   - **A flaky, slow, or badly-isolated test is a test-infrastructure symptom,
+     not a production risk — until you have evidence it hides a real defect.**
+     Tier it by the harness cost, and prefer *fixing* it over escalating it:
+     - Flakiness / nondeterminism whose cause is the *harness* (a live network
+       dependency, a fixed-`sleep` timing race, a shared mutable fixture, a real
+       clock/animation) → typically **MEDIUM**, **escalate = no**. It is critical
+       tech debt worth fixing (isolate the boundary, use a deterministic wait),
+       but a slow or flaky *test* does not by itself endanger production, so it is
+       not an escalation.
+     - A suite that is **slow but fully deterministic** (reliably green, no
+       intermittent failures) is a CI-speed / toil concern → typically **LOW**,
+       **escalate = no**. Recommend speeding it up; do not inflate it.
+   - **Escalate a flaky test only when the flakiness is a *symptom of a real
+     production defect*** — e.g. the nondeterminism comes from an unsynchronized
+     race, lost update, or ordering bug in the *product* code, and a retry loop
+     would paper over it. That is **HIGH / escalate = yes**: the bug, not the
+     test, is the risk.
+   - **Do not let a high-stakes domain word inflate the tier when it appears only
+     in a *fixture*.** A "payments sandbox", an auth *stub*, or a mock production
+     hostname used by a test is test scaffolding — tier by what the *change*
+     actually risks in production, not by the vocabulary of the fixture. A flaky
+     E2E that merely *talks to* a payments sandbox is a MEDIUM isolation problem,
+     not a HIGH money-path risk.
+   - **Escalation is independent of the risk tier — decide each separately.**
+     Escalate whenever a gap needs a maintainer's attention before merge; that
+     can happen at MEDIUM, not only at HIGH. **Reserve HIGH for a real
+     production risk left unguarded** — an untested money/auth/data path, a
+     destructive migration with no rollback test, or the
+     flakiness-masks-a-real-defect case above: gaps with substantial production
+     blast radius. A **coverage-quality** gap such as meaningless assertions
+     that fake coverage on a real behavior is **MEDIUM / escalate = yes** — the
+     tests are hollow and must be fixed before merge, but the shipped behavior
+     is not itself high blast-radius, so it does not warrant HIGH. Never wave
+     through a real risk as LOW just to avoid over-escalating; the goal is the
+     right call on both tier and escalation.
+
 ## Output — how you deliver the advisory
 
 Print the comment body **between these exact sentinel lines**, each alone on its

tests/test_qa_lead_holdout_calibration.bats

@@ -0,0 +1,122 @@
+#!/usr/bin/env bats
+# Regression guard for the qa-lead risk-tiering recalibration (#1698).
+#
+# #1698 found qa-lead over-escalates a test-infrastructure flakiness symptom to
+# HIGH/escalate where the reference is MEDIUM/no. The fix lives in the advisory
+# prompt's risk-tiering guidance (prompts/qa-lead/advisory.md), but its calibration
+# is characterised by held-out cases that separate three distinct signals:
+#
+#   - flakiness with no production impact          -> MEDIUM / escalate no
+#   - flakiness that MASKS a real production race  -> HIGH   / escalate yes
+#   - a slow-but-deterministic suite               -> LOW    / escalate no
+#
+# This test pins those three additions (AC #1) and asserts the six pre-existing
+# held-out cases keep their expected escalate/risk verdicts, enforcing AC #2's
+# "holdout stays byte-stable except for AC #1's additions" — in particular that
+# the flaky-network-e2e reference stays MEDIUM/no and payment-happy-only stays
+# HIGH/yes (the bias must not be "fixed" by weakening the references).
+
+setup() {
+  ROOT="$(cd "$BATS_TEST_DIRNAME/.." && pwd)"
+  HOLDOUT="$ROOT/evals/qa-lead/holdout/cases.jsonl"
+}
+
+# assert_case <id> <escalate true|false> <risk LOW|MEDIUM|HIGH> [expected_recommend]
+#
+# When a fourth argument is given, the case's `recommend` must match it exactly —
+# used to pin the six pre-existing references (AC #2's byte-stable requirement)
+# so an unrelated recommendation cannot silently pass. When omitted, `recommend`
+# only has to be non-empty — used for the three new additions (AC #1).
+assert_case() {
+  local id="$1" escalate="$2" risk="$3" want_rec="${4-}"
+  run python3 -c '
+import json, sys
+path, cid, want_esc, want_risk = sys.argv[1:5]
+want_rec = sys.argv[5] if len(sys.argv) > 5 else None
+want_esc = want_esc == "true"
+found = None
+for line in open(path, encoding="utf-8"):
+    line = line.strip()
+    if not line:
+        continue
+    case = json.loads(line)
+    if case.get("id") == cid:
+        found = case
+        break
+if found is None:
+    print(f"missing case id: {cid}")
+    sys.exit(1)
+exp = found.get("expected", {})
+esc_val = exp.get("escalate")
+if esc_val is not want_esc:
+    print(f"{cid}: escalate={esc_val} want {want_esc}")
+    sys.exit(1)
+risk_val = exp.get("risk")
+if risk_val != want_risk:
+    print(f"{cid}: risk={risk_val} want {want_risk}")
+    sys.exit(1)
+rec_val = str(exp.get("recommend", "")).strip()
+if not rec_val:
+    print(f"{cid}: missing non-empty recommend")
+    sys.exit(1)
+if want_rec is not None and rec_val != want_rec:
+    print(f"{cid}: recommend mismatch\n  got:  {rec_val}\n  want: {want_rec}")
+    sys.exit(1)
+print("ok")
+' "$HOLDOUT" "$id" "$escalate" "$risk" ${want_rec:+"$want_rec"}
+  [ "$status" -eq 0 ] || { echo "$output"; return 1; }
+}
+
+# --- AC #1: the three new characterization cases ------------------------------
+
+@test "flaky UI/timing test with no production impact is MEDIUM / no-escalate" {
+  assert_case "qa-lead-hold-flaky-ui-timing" false MEDIUM
+}
+
+@test "flakiness masking a real production race is HIGH / escalate" {
+  assert_case "qa-lead-hold-flaky-masks-real-race" true HIGH
+}
+
+@test "slow-but-deterministic suite is LOW / no-escalate" {
+  assert_case "qa-lead-hold-slow-deterministic-suite" false LOW
+}
+
+# --- AC #2: the six pre-existing references are unchanged ----------------------
+
+@test "flaky-network-e2e reference stays MEDIUM / no-escalate" {
+  assert_case "qa-lead-hold-flaky-network-e2e" false MEDIUM \
+    "stub/mock the external dependency at the boundary (or contract-test it) so CI is deterministic"
+}
+
+@test "payment-happy-only reference stays HIGH / escalate" {
+  assert_case "qa-lead-hold-payment-happy-only" true HIGH \
+    "add declined, duplicate-submit (idempotency), and refund cases before merge — money paths are high-impact"
+}
+
+@test "auth-no-contract reference stays HIGH / escalate" {
+  assert_case "qa-lead-hold-auth-no-contract" true HIGH \
+    "add negative-path API tests (expired/invalid/replay) and a Pact contract test before merge"
+}
+
+@test "migration-no-rollback-test reference stays HIGH / escalate" {
+  assert_case "qa-lead-hold-migration-no-rollback-test" true HIGH \
+    "add rollback + idempotency tests and capture a migration-duration NFR estimate"
+}
+
+@test "coverage-gaming reference stays MEDIUM / escalate" {
+  assert_case "qa-lead-hold-coverage-gaming" true MEDIUM \
+    "coverage is gamed; require behavioral assertions on outputs/side-effects, not exception-free execution"
+}
+
+@test "well-tested-refactor reference stays LOW / no-escalate" {
+  assert_case "qa-lead-hold-well-tested-refactor" false LOW \
+    "no additional tests required; existing coverage is sufficient"
+}
+
+# --- The committed holdout stays schema-valid with the additions --------------
+
+@test "committed qa-lead holdout validates against the case schema" {
+  run python3 "$ROOT/evals/validate-cases.py" "$HOLDOUT" "$ROOT/evals/case.schema.json"
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"OK"* ]]
+}