From 93a65eff2362eeb96cbf9499fbc2175814a00e23 Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Mon, 13 Jul 2026 23:37:28 -0500 Subject: [PATCH 1/3] test(dev-lead): guard the #443 concurrency fix (cancel-in-progress: false) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dev-lead multi-label issue-pickup race (#443) was fixed by #450's per-issue/ per-PR concurrency lanes with cancel-in-progress:false — same-lane labeled events queue behind the active run instead of cancelling it, so the dev-lead-triggered pickup always finishes. Add a regression guard that fails if anyone reintroduces cancel-in-progress:true or drops the per-issue lane. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Juznz5V6su81ffSND8fg7s --- tests/test_dev_lead_concurrency_guard.bats | 27 ++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 tests/test_dev_lead_concurrency_guard.bats diff --git a/tests/test_dev_lead_concurrency_guard.bats b/tests/test_dev_lead_concurrency_guard.bats new file mode 100644 index 000000000..f40b79edd --- /dev/null +++ b/tests/test_dev_lead_concurrency_guard.bats @@ -0,0 +1,27 @@ +#!/usr/bin/env bats +# Regression guard for petry-projects/.github#443 (and #402/#450). +# +# The dev-lead issue-pickup race: GitHub fires one issues:labeled event per label +# added in a single API call. If the reusable's per-issue concurrency used +# cancel-in-progress:true, a later (non-dev-lead) labeled event could CANCEL the +# run triggered by the dev-lead label before it dispatched — a "successful" +# workflow that opened no PR. The fix (#450) is separate per-issue/per-PR lanes +# with cancel-in-progress:FALSE, so same-lane events queue behind the active run +# and the pickup always finishes. These asserts fail loudly if anyone flips it back. + +REUSABLE=".github/workflows/dev-lead-reusable.yml" + +@test "dev-lead reusable concurrency is cancel-in-progress: false (#443/#450)" { + run grep -E '^\s*cancel-in-progress:\s*false\s*$' "$REUSABLE" + [ "$status" -eq 0 ] +} + +@test "dev-lead reusable has NO cancel-in-progress: true (would re-open the #443 race)" { + run grep -E '^\s*cancel-in-progress:\s*true\s*$' "$REUSABLE" + [ "$status" -ne 0 ] +} + +@test "dev-lead reusable routes issue events to a per-issue lane (#402 lanes)" { + run grep -F "format('dev-lead-issue-{0}', github.event.issue.number)" "$REUSABLE" + [ "$status" -eq 0 ] +} From a724ca4a4a096de8146f321e628efead12573e17 Mon Sep 17 00:00:00 2001 From: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com> Date: Tue, 14 Jul 2026 04:49:41 +0000 Subject: [PATCH 2/3] chore: dev-lead update (review-changes) [skip ci-relay] --- .github/workflows/lint.yml | 4 +- .../deep-review-baseline/PROVENANCE.md | 107 ++++++++++++++++++ .../frozen-baseline-2026-07.json | 49 ++++++++ tests/test_deep_review_baseline.bats | 97 ++++++++++++++++ tests/test_dev_lead_concurrency_guard.bats | 2 +- 5 files changed, 257 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/deep-review-baseline/PROVENANCE.md create mode 100644 tests/fixtures/deep-review-baseline/frozen-baseline-2026-07.json create mode 100644 tests/test_deep_review_baseline.bats diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 57d813a73..5e368086c 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -98,6 +98,7 @@ jobs: tests/test_downstream_impact_triage_inline.bats \ tests/test_downstream_impact_regression.bats \ tests/test_et_baseline_regression.bats \ + tests/test_deep_review_baseline.bats \ tests/test_safety_checks.bats \ tests/test_safety_checks_triage_inline.bats \ tests/test_prefetch_context.bats \ @@ -108,7 +109,8 @@ jobs: tests/test_review_one_pr_ci_pending.bats \ tests/test_release_registry.bats \ tests/test_readme_refresh_helpers.bats \ - tests/test_validate_personas.bats + tests/test_validate_personas.bats \ + tests/test_dev_lead_concurrency_guard.bats template-drift: # Drift guard (#969): fail when a file committed in petry-projects/repo-template diff --git a/tests/fixtures/deep-review-baseline/PROVENANCE.md b/tests/fixtures/deep-review-baseline/PROVENANCE.md new file mode 100644 index 000000000..30bb79fa4 --- /dev/null +++ b/tests/fixtures/deep-review-baseline/PROVENANCE.md @@ -0,0 +1,107 @@ +# Frozen deep-review bug-hunter baseline — provenance + +**Issue:** #1089 (Phase 1) · **Epic:** #1088 — PR-review world-class bug hunter +**Artifact:** `frozen-baseline-2026-07.json` · **Status:** FROZEN / read-only + +## What this is + +The immutable **before-numbers** for the bug-hunter initiative. Every downstream enhancement (#1090 semantic symbol context, #1091 issue-type specialist +routing, #1092 agentic validation, #1093 merged-PR few-shot) must prove **no regression** +against these figures, and the Phase-4 convergence gate (#1094) consumes them as +the fixed reference before any `LIVE_MODE` flip. The numbers here must never move +by prompt-tuning — otherwise a later "win" could be manufactured by shifting the +goalpost rather than by actually finding more bugs at bounded cost. + +The baseline captures the three signals named in the epic's success metric and +cost cap: + +| Metric | Value | Status | +|--------|-------|--------| +| Median escalated-review (deep-tier) ET | **343,068.25** | **frozen (real telemetry)** | +| Deep-review held-out eval aggregate score (llm-judge, threshold 0.7) | `null` | pending — credentialed capture | +| False-positive rate (`finding_verification` records) | `null` | unavailable — emitter not implemented | + +## 1. Median escalated-review ET — frozen, real, not synthesized + +- **What it is.** The median per-call Effective Tokens (ET) of the **deep** review + tier — the tier `triage` escalates into, i.e. the "escalated review" the epic's + cost cap governs (*"median ET per escalated review must stay ≤ 1.5× the Phase-1 + frozen baseline"*). Median (not mean) so a single pathological review cannot + skew the goalpost. +- **Source.** Computed directly from the 10 genuine `deep`-tier records already + committed and frozen in + [`tests/fixtures/et-baseline/pre-change-baseline-2026-07.jsonl`](../et-baseline/pre-change-baseline-2026-07.jsonl) + (issue #1102). Those records were extracted verbatim from the live `pr-review` + pipeline's `token-usage-` CI artifacts — the same per-call JSONL + `scripts/token_report.sh` prices. This artifact does **not** duplicate that + data; it references it and freezes the derived median, so the two artifacts + cannot silently diverge (the regression guard recomputes the median from the + source and fails on any mismatch). +- **Dated window:** 2026-07-03 → 2026-07-10 (UTC), inclusive. +- **Source run IDs (deep tier, `pr-review`):** `28680743571`, `28680719157`, + `28684575826`, `28690647414`, `28708785471`, `28712486134`, `28945288331`, + `28945290473`, `29004496155`, `29060937544`. +- **Reproduce:** + ```bash + jq -s 'map(select(.tier=="deep").et) | sort as $s | ($s|length) as $n + | if $n%2==1 then $s[($n/2|floor)] else (($s[$n/2-1]+$s[$n/2])/2) end' \ + tests/fixtures/et-baseline/pre-change-baseline-2026-07.jsonl + # => 343068.25 + ``` +- **Cost cap derived from it:** 1.5× → **514,602.375** ET is the ceiling a + downstream story's median escalated-review ET must stay under. + +## 2. Deep-review held-out eval score — pending credentialed capture + +`scripts/evals/run-eval.sh deep-review` scores the five held-out cases in +`evals/deep-review/holdout/cases.jsonl` with the CODEOWNER-gated llm-judge +(`evals/judge.md`, `pass_threshold` 0.7). It makes **live model calls** through +the engine (`run_triage`), so it cannot be scored in an un-credentialed context: +the dev-lead sandbox reports *"Not logged in"*, so every case is an infra failure +and `run-eval.sh` exits 2 (un-scored, by design — a throttle must not open a false +held-out regression, #920). + +The value is therefore left `null` with `status: pending-credentialed-capture` and +the exact reproduce command. It must be frozen by a **credentialed CI run through +an explicit CODEOWNER-reviewed PR** — the automated skill-proposer identity must +never freeze the goalpost it is later graded against (`holdout-guard.yml`, #692). +Until it is frozen, the Phase-4 gate (#1094) captures a same-run baseline at gate +time and compares the candidate against it, which is strictly sound (same engine, +same judge, same window). + +## 3. False-positive rate — unavailable (emitter not implemented) + +The epic's success metric measures FP-rate from `kind:"finding_verification"` +records (`severity_before`/`severity_after`/`outcome`) said to be emitted by +`emit_verification_record()` in `scripts/lib/token-metrics.sh`. **That emitter +does not exist.** `finding_verification` is a *documented schema only* — it is +referenced by `tests/token_report.bats` (story #843, which pins that +`token_report.sh` **excludes** such records from cost) but no production code +emits it, and no such records exist in the telemetry window. + +A real FP-rate baseline consequently **cannot be captured yet**: with zero +`finding_verification` records the rate is undefined. Standing up the emitter is +itself a prioritized gap — see the audit +([`docs/initiatives/pr-review-bughunter-audit.md`](../../../docs/initiatives/pr-review-bughunter-audit.md)), +where it maps to **#1092** (agentic validation, which produces the +severity-downgrade/refute outcomes the FP-rate is computed from). The story that +first emits these records freezes this number via a CODEOWNER-reviewed PR. + +## Why the numbers cannot drift + +- The frozen ET median is priced at the model-multiplier **in effect on each + record's own date** (`model-pricing.tsv` selection rule), and every source + record is dated in a fixed past window — appending a future price row cannot + retroactively move it. +- [`tests/test_deep_review_baseline.bats`](../../test_deep_review_baseline.bats) + pins the exact median, the record count, and recomputes the median from the + referenced source JSONL, and asserts the two pending metrics keep their honest + `null` status — so any edit (including a silent flip of a pending metric to an + unreviewed number) fails CI and must be an explicit, reviewable fixture change. + +## Immutability controls + +- Owner-locked in `.github/CODEOWNERS` (`/tests/fixtures/deep-review-baseline/`). +- Protected from silent deletion by `test-deletion-guard.yml` (anything under + `tests/` requires the `ack-test-deletion` label to delete). +- Pinned by the regression guard above (registered in `lint.yml`). diff --git a/tests/fixtures/deep-review-baseline/frozen-baseline-2026-07.json b/tests/fixtures/deep-review-baseline/frozen-baseline-2026-07.json new file mode 100644 index 000000000..f9a77af71 --- /dev/null +++ b/tests/fixtures/deep-review-baseline/frozen-baseline-2026-07.json @@ -0,0 +1,49 @@ +{ + "status": "FROZEN", + "issue": 1089, + "epic": 1088, + "captured_at": "2026-07-14", + "description": "Immutable Phase-1 quality/cost baseline for the deep-review bug-hunter initiative (epic #1088). Downstream stories #1090-#1093 prove 'no regression' against these numbers; the Phase-4 gate #1094 consumes them as the before-reference. Do not edit without a CODEOWNER review — the pin in tests/test_deep_review_baseline.bats fails CI on any silent change.", + "metrics": { + "median_escalated_review_et": { + "value": 343068.25, + "unit": "effective_tokens", + "status": "frozen", + "tier": "deep", + "model": "claude-opus-4-8", + "record_count": 10, + "window": { "from": "2026-07-03", "to": "2026-07-10" }, + "source": "tests/fixtures/et-baseline/pre-change-baseline-2026-07.jsonl", + "source_run_ids": [ + "28680743571", "28680719157", "28684575826", "28690647414", + "28708785471", "28712486134", "28945288331", "28945290473", + "29004496155", "29060937544" + ], + "reproduce": "jq -s 'map(select(.tier==\"deep\").et)|sort as $s|($s|length) as $n|if $n%2==1 then $s[($n/2|floor)] else (($s[$n/2-1]+$s[$n/2])/2) end' tests/fixtures/et-baseline/pre-change-baseline-2026-07.jsonl", + "note": "The deep tier is the escalated-review tier (triage escalates into it). Median of the 10 real deep-tier per-call ET records already committed in the et-baseline fixture and priced by scripts/token_report.sh. Epic #1088 cost cap: median ET per escalated review must stay <= 1.5x this value (514602.375)." + }, + "deep_review_holdout_eval_score": { + "value": null, + "status": "pending-credentialed-capture", + "scorer": "llm-judge", + "judge": "evals/judge.md", + "pass_threshold": 0.7, + "holdout": "evals/deep-review/holdout/cases.jsonl", + "reproduce": "scripts/evals/run-eval.sh deep-review", + "note": "run-eval.sh deep-review makes live model calls through the engine (run_triage). It cannot be scored in an un-credentialed context (the dev-lead sandbox reports 'Not logged in', so every case is an infra failure -> exit 2, un-scored). This aggregate score must be frozen by a credentialed CI run via an explicit CODEOWNER-reviewed PR; a proposer-identity run must never freeze its own goalpost (holdout-guard.yml, #692). Until then downstream 'no regression' checks compare a fresh credentialed run against a same-run baseline captured at gate time (#1094)." + }, + "finding_verification_fp_rate": { + "value": null, + "status": "unavailable-no-emitter", + "signal_kind": "finding_verification", + "expected_source": "scripts/lib/token-metrics.sh", + "reproduce": "n/a until the emitter lands", + "note": "The false-positive signal (kind:\"finding_verification\" records with severity_before/after + outcome) is a DOCUMENTED SCHEMA ONLY (tests/token_report.bats, story #843) — no emit_verification_record() exists in scripts/lib/token-metrics.sh and no such records are present in the telemetry window. A real FP-rate baseline therefore cannot be captured yet; standing it up is itself a prioritized gap (see docs/initiatives/pr-review-bughunter-audit.md, mapped to #1092). Baseline = 0 finding_verification records -> FP-rate undefined. The story that first emits these records freezes this number via a CODEOWNER-reviewed PR." + } + }, + "references": { + "audit": "docs/initiatives/pr-review-bughunter-audit.md", + "immutability_pattern": "tests/fixtures/et-baseline/PROVENANCE.md", + "regression_guard": "tests/test_deep_review_baseline.bats" + } +} diff --git a/tests/test_deep_review_baseline.bats b/tests/test_deep_review_baseline.bats new file mode 100644 index 000000000..ce39c0493 --- /dev/null +++ b/tests/test_deep_review_baseline.bats @@ -0,0 +1,97 @@ +#!/usr/bin/env bats +# Frozen deep-review baseline regression guard (issue #1089, epic #1088). +# +# Phase 1 of the "world-class bug hunter" initiative freezes an IMMUTABLE +# quality/cost baseline so every downstream enhancement (#1090–#1093) can prove +# "no regression" against a fixed reference and the Phase-4 gate (#1094) has a +# stable before-number. This guard PINS the committed baseline artifact: +# - it exists and is valid JSON; +# - the frozen median escalated-review (deep-tier) ET matches an exact value, +# recomputed from the SAME real telemetry the Token Cost Observatory prices +# (tests/fixtures/et-baseline/pre-change-baseline-2026-07.jsonl) so the stored +# number can never silently drift from its source records; +# - the two metrics that cannot be captured in an un-credentialed / pre-emitter +# context (the llm-judge holdout eval score, and the finding_verification +# false-positive rate) keep their documented pending/unavailable status, so a +# silent flip to an unreviewed number fails CI. +# +# Mirrors tests/test_et_baseline_regression.bats: any edit to the frozen numbers +# fails CI and must be an explicit, reviewable fixture change visible in the diff. +# That is the whole point — the goalpost cannot move without a CODEOWNER review. +# +# Run with: bats tests/test_deep_review_baseline.bats + +setup() { + FIXDIR="$(dirname "$BATS_TEST_FILENAME")/fixtures/deep-review-baseline" + BASELINE="$FIXDIR/frozen-baseline-2026-07.json" + ET_SOURCE="$(dirname "$BATS_TEST_FILENAME")/fixtures/et-baseline/pre-change-baseline-2026-07.jsonl" +} + +# --------------------------------------------------------------------------- +# The frozen artifact and its provenance must exist and be well-formed (a +# deleted/renamed artifact must not silently turn the guard into a no-op). +# --------------------------------------------------------------------------- + +@test "deep-review baseline artifact exists" { + [ -f "$BASELINE" ] +} + +@test "deep-review baseline provenance exists" { + [ -f "$FIXDIR/PROVENANCE.md" ] +} + +@test "deep-review baseline is valid JSON" { + [ -s "$BASELINE" ] + jq empty "$BASELINE" +} + +@test "baseline is marked FROZEN and bound to issue #1089 / epic #1088" { + [ "$(jq -r '.status' "$BASELINE")" = "FROZEN" ] + [ "$(jq -r '.issue' "$BASELINE")" = "1089" ] + [ "$(jq -r '.epic' "$BASELINE")" = "1088" ] +} + +# --------------------------------------------------------------------------- +# Frozen cost baseline: the median escalated-review (deep-tier) ET is pinned to +# an exact value AND recomputed from the referenced real telemetry, so the stored +# figure can never diverge from its source records. +# --------------------------------------------------------------------------- + +@test "median escalated-review ET is frozen to the exact pinned value" { + local v + v="$(jq -r '.metrics?.median_escalated_review_et?.value' "$BASELINE")" + [ "$v" = "343068.25" ] + [ "$(jq -r '.metrics?.median_escalated_review_et?.status' "$BASELINE")" = "frozen" ] + [ "$(jq -r '.metrics?.median_escalated_review_et?.record_count' "$BASELINE")" = "10" ] +} + +@test "stored median recomputes from the referenced et-baseline telemetry" { + [ -f "$ET_SOURCE" ] + local recomputed stored count + recomputed="$(jq -s 'map(select(.tier=="deep").et) | sort as $s | ($s|length) as $n + | if $n % 2 == 1 then $s[($n/2|floor)] else (($s[$n/2-1] + $s[$n/2]) / 2) end' "$ET_SOURCE")" + stored="$(jq -r '.metrics?.median_escalated_review_et?.value' "$BASELINE")" + # Numeric compare (avoid trailing-zero string mismatches). + awk -v a="$recomputed" -v b="$stored" 'BEGIN { exit (a + 0 == b + 0) ? 0 : 1 }' + # The pinned record_count must match the real source too. + count="$(jq -s 'map(select(.tier=="deep")) | length' "$ET_SOURCE")" + [ "$count" = "$(jq -r '.metrics?.median_escalated_review_et?.record_count' "$BASELINE")" ] +} + +# --------------------------------------------------------------------------- +# The two not-yet-capturable metrics must keep their documented, honest status +# (no fabricated number). A silent flip to a real value fails CI so that the +# first genuine capture goes through an explicit CODEOWNER-reviewed diff. +# --------------------------------------------------------------------------- + +@test "deep-review holdout eval score stays pending (needs credentialed capture)" { + [ "$(jq -r '.metrics?.deep_review_holdout_eval_score?.value' "$BASELINE")" = "null" ] + [ "$(jq -r '.metrics?.deep_review_holdout_eval_score?.status' "$BASELINE")" = "pending-credentialed-capture" ] + # The reproducible capture command must be recorded so the number is fillable. + [ "$(jq -r '.metrics?.deep_review_holdout_eval_score?.reproduce' "$BASELINE")" != "null" ] +} + +@test "finding_verification FP-rate stays unavailable (emitter not implemented)" { + [ "$(jq -r '.metrics?.finding_verification_fp_rate?.value' "$BASELINE")" = "null" ] + [ "$(jq -r '.metrics?.finding_verification_fp_rate?.status' "$BASELINE")" = "unavailable-no-emitter" ] +} diff --git a/tests/test_dev_lead_concurrency_guard.bats b/tests/test_dev_lead_concurrency_guard.bats index f40b79edd..1b36edf82 100644 --- a/tests/test_dev_lead_concurrency_guard.bats +++ b/tests/test_dev_lead_concurrency_guard.bats @@ -18,7 +18,7 @@ REUSABLE=".github/workflows/dev-lead-reusable.yml" @test "dev-lead reusable has NO cancel-in-progress: true (would re-open the #443 race)" { run grep -E '^\s*cancel-in-progress:\s*true\s*$' "$REUSABLE" - [ "$status" -ne 0 ] + [ "$status" -eq 1 ] } @test "dev-lead reusable routes issue events to a per-issue lane (#402 lanes)" { From 20efbd45e16498ba3c2046c022b6070f91610243 Mon Sep 17 00:00:00 2001 From: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com> Date: Tue, 14 Jul 2026 04:54:33 +0000 Subject: [PATCH 3/3] chore: dev-lead update (review-changes) [skip ci-relay] --- tests/test_dev_lead_concurrency_guard.bats | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_dev_lead_concurrency_guard.bats b/tests/test_dev_lead_concurrency_guard.bats index 1b36edf82..d37d0442e 100644 --- a/tests/test_dev_lead_concurrency_guard.bats +++ b/tests/test_dev_lead_concurrency_guard.bats @@ -6,18 +6,18 @@ # cancel-in-progress:true, a later (non-dev-lead) labeled event could CANCEL the # run triggered by the dev-lead label before it dispatched — a "successful" # workflow that opened no PR. The fix (#450) is separate per-issue/per-PR lanes -# with cancel-in-progress:FALSE, so same-lane events queue behind the active run +# with cancel-in-progress: false, so same-lane events queue behind the active run # and the pickup always finishes. These asserts fail loudly if anyone flips it back. REUSABLE=".github/workflows/dev-lead-reusable.yml" @test "dev-lead reusable concurrency is cancel-in-progress: false (#443/#450)" { - run grep -E '^\s*cancel-in-progress:\s*false\s*$' "$REUSABLE" + run grep -E '^[[:space:]]*cancel-in-progress:[[:space:]]*false[[:space:]]*$' "$REUSABLE" [ "$status" -eq 0 ] } @test "dev-lead reusable has NO cancel-in-progress: true (would re-open the #443 race)" { - run grep -E '^\s*cancel-in-progress:\s*true\s*$' "$REUSABLE" + run grep -E '^[[:space:]]*cancel-in-progress:[[:space:]]*true[[:space:]]*$' "$REUSABLE" [ "$status" -eq 1 ] }