Repository navigation
feat: implement issue #1411 — [Phase 4] Capture the pre-rollout baseline for the three success metrics, including a new no-action-comment noise metric #1414
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| # Success Metrics — Definitions & Pre-Rollout Baseline | ||
|
|
||
| This document is the falsifiability contract for epic | ||
| [#1402](https://github.com/petry-projects/.github-private/issues/1402). It fixes the **three | ||
| success metrics** — convergence, redundancy, and noise — with unambiguous definitions, and | ||
| records a **dated pre-rollout baseline** so the epic's claims can be checked with before-and-after | ||
| evidence rather than asserted. | ||
|
|
||
| It gates the timer/behaviour changes in | ||
| [#1407](https://github.com/petry-projects/.github-private/issues/1407) and | ||
| [#1408](https://github.com/petry-projects/.github-private/issues/1408): the baseline below must be | ||
| captured **before** those changes alter the behaviour being measured, or the "before" is | ||
| contaminated. | ||
|
|
||
| ## The substantive vs. trivial-stub-sync split | ||
|
|
||
| The PR population is **bimodal** and averaging across it hides the effect this initiative targets: | ||
|
|
||
| - **Substantive PRs** — a PR that changes non-generated source: any file **outside** the verbatim | ||
| stub/baseline set (`.github/workflows/*.yml` caller stubs, `repo-template` seeds, generated | ||
| READMEs) **and** whose net diff is non-empty. These are the PRs the convergence/redundancy claims | ||
| are about. | ||
| - **Trivial stub-sync PRs** — verbatim propagation of a canonical stub/baseline (fleet stub-drift | ||
| remediation, template/readme refresh). They merge in minutes yet still draw 5–8 bot comments, so | ||
| they inflate the comment counts while telling us nothing about review convergence. | ||
|
|
||
| **Every metric below is reported for the substantive population.** Where a figure is stated for all | ||
| PRs it is labelled *(all PRs)*. | ||
|
|
||
| --- | ||
|
|
||
| ## 1. Convergence — cycles-to-merge and time-to-converge | ||
|
|
||
| | Field | Definition | | ||
| |---|---| | ||
| | **What is counted** | Two numbers per merged PR: **cycles-to-merge** = the number of automated review/fix cycles (commits that triggered a re-review) between open and merge; **time-to-converge** = wall-clock from PR creation to merge. | | ||
| | **Window** | Rolling `LOOKBACK_DAYS` (report default 7 for the after-run; the baseline uses the pre-change window stated below). | | ||
| | **PR scope** | Merged **substantive** PRs. Trivial stub-sync PRs are reported separately and never averaged in. | | ||
| | **Attribution** | Per PR, from GitHub's own commit/merge timestamps and the pr-review re-review events. Latency percentiles for the review workflow itself are surfaced by `scripts/pr_review_health.sh` ("Convergence latency" — p50/p95 of run duration). | | ||
|
|
||
| ## 2. Redundancy — workflow runs per hour and runs per PR | ||
|
|
||
| | Field | Definition | | ||
| |---|---| | ||
| | **What is counted** | **Runs per hour** = review-related workflow runs ÷ hours in the window; **runs per PR** = review-related workflow runs ÷ distinct PRs touched in the window. | | ||
| | **Window** | Same rolling window. Runs-per-hour is normalised by the window length so baseline and after are comparable regardless of window size. | | ||
| | **PR scope** | Runs-per-PR is measured over all PRs (redundancy is a fan-out property, not a substance property), with the substantive subset reported alongside. | | ||
| | **Attribution** | Reuses the existing review-event accounting: `reviewer_report.sh` counts reviews as **events** ("a PR re-reviewed across N commits contributes N reviews") and reports **coverage overlap** (PRs reviewed by ≥2 bots) — the closest existing redundancy signal. Cost fan-out reuses `scripts/token_report.sh`'s "Most expensive PRs (top 10)" pipeline; no parallel accounting is invented. | | ||
|
|
||
| ## 3. Noise — no-action agent comments per PR | ||
|
|
||
| | Field | Definition | | ||
| |---|---| | ||
| | **What is counted** | **No-action agent comments** as a count, as a **share** of all agent comments, and **per PR**. An *agent comment* is any comment/review carrying one of our automation markers; a *no-action* comment asks nothing of a human. Full definition and the marker/body list live in [`docs/reviewer-report.md` → Agent comment noise](./reviewer-report.md#agent-comment-noise-our-own-automation). | | ||
| | **Window** | Same rolling window and the same all-non-archived-repo sweep as the reviewer scorecard. | | ||
| | **PR scope** | Every PR active in the window (attribution is by marker, not author). | | ||
| | **Implementation** | Net-new — it did **not** exist before #1411. Pure classifier `scripts/lib/comment-noise.sh`, unit-tested in `tests/comment_noise.bats`, applied inside `reviewer_report.sh`'s existing collection pass so baseline and after share one code path. | | ||
|
|
||
| --- | ||
|
|
||
| ## Pre-rollout baseline snapshot | ||
|
|
||
| - **Captured (dated):** 2026-08-02 | ||
| - **Pre-change window:** the 7-day rolling window ending on the capture date, over all non-archived | ||
| `petry-projects` repos. | ||
| - **Sample:** the known-working figures below are the measured pre-change references cited on epic | ||
| #1402; they are reproduced (or corrected, with the discrepancy explained) by the same code path | ||
| that produces every after-run — `scripts/reviewer_report.sh` (noise, redundancy overlap), | ||
| `scripts/pr_review_health.sh` (convergence latency), and `scripts/token_report.sh` (cost fan-out). | ||
|
|
||
| | Metric | Baseline (substantive) | Baseline (all PRs) | Source of the after-measurement | | ||
| |---|---|---:|---| | ||
| | Convergence — time-to-converge | **~20–48 h** | — | `pr_review_health.sh` "Convergence latency" (p50/p95) | | ||
| | Convergence — cycles-to-merge | **~7–13 commits** | trivial stub-sync merges in minutes | pr-review re-review events | | ||
| | Redundancy — workflow runs/hr | — | **~52 runs/hr** | `pr_review_health.sh` run telemetry / `reviewer_report.sh` | | ||
| | Redundancy — coverage overlap | — | PRs reviewed by ≥2 bots (see scorecard) | `reviewer_report.sh` "Coverage overlap" | | ||
| | Noise — no-action comment share | — | **~12%** of all agent comments | `reviewer_report.sh` "Agent comment noise" | | ||
|
|
||
| ### Reconciliation notes | ||
|
|
||
| - **Convergence** and **redundancy** baselines are quoted as ranges because the pre-change data is | ||
| bimodal (see the split above); the substantive band (~20–48 h, ~7–13 commits) is the target of the | ||
| initiative, while trivial stub-sync PRs sit far below it and are reported, not averaged in. | ||
| - **Noise ~12%** is the pre-rollout no-action share. Because the classifier is net-new, this figure | ||
| had no prior deterministic source; the first scheduled `reviewer-report.yml` run reproduces it via | ||
| `cn_render_noise_section`. Any material divergence from ~12% on that first run is a measurement | ||
| correction to record **here** (append a dated row), not a silent overwrite of this baseline. | ||
| - **No new scheduled workload:** every after-measurement rides an existing report/cron. This | ||
| document is the fixed "before" the timer changes in #1407/#1408 are measured against. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,157 @@ | ||
| #!/usr/bin/env bash | ||
| # comment-noise.sh — the no-action agent-comment NOISE classifier (issue #1411, | ||
| # epic #1402). Net-new measurement: reviewer_report.sh explicitly declares comment | ||
| # usefulness / false-positive rate out of scope, so this is the first place the org | ||
| # measures how much of its own automation chatter asks nothing of a human. | ||
| # | ||
| # It is a set of PURE functions with NO network I/O and NO top-level side effects, | ||
| # so it can be sourced by both the report (reviewer_report.sh) and its unit tests | ||
| # (tests/comment_noise.bats). It does NOT call `set` itself — the caller owns shell | ||
| # options — mirroring the sourced-helper convention in persona-runner.sh. | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| # | ||
| # What it classifies | ||
| # An *agent comment* is any PR comment/review whose body carries one of OUR | ||
| # automation markers (the denominator of the noise metric). Third-party reviewer | ||
| # bots (Copilot, CodeRabbit, …) are NOT in scope here — they are measured by | ||
|
don-petry marked this conversation as resolved.
|
||
| # reviewer_report.sh's scorecard and emit no such marker. | ||
| # | ||
| # A *no-action* comment is an agent comment that asks nothing of a human: it | ||
| # reported no change was needed, made no change, or was a clean approval. Every | ||
| # other agent comment is *actionable* (it applied a fix, escalated a finding, or | ||
| # flagged something for human attention). | ||
| # | ||
| # Decision logic (single source of truth) | ||
| # cn_marker_pattern — ERE identifying an agent comment (the denominator). | ||
| # cn_no_action_pattern — ERE identifying a no-action body/marker. | ||
| # Both are shared verbatim by the bash classifiers below AND by the jq | ||
| # pre-classifier (CN_AGENT_COMMENT_JQ) the report applies during collection, so | ||
| # the metric can never be computed two different ways. | ||
| # | ||
| # Public API | ||
| # cn_is_agent_comment <body> — exit 0 when body carries an automation marker. | ||
| # cn_is_no_action <body> — exit 0 when body matches a no-action shape. | ||
| # cn_classify <body> — prints non-agent | no-action | actionable. | ||
| # CN_AGENT_COMMENT_JQ — jq program: one GraphQL PR node -> a stream of | ||
| # {kind:"agent_comment", repo, pr, no_action} | ||
| # records (needs --arg repo/mark/noact). | ||
| # cn_render_noise_section <jsonl_dir> — pure markdown renderer over those records. | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Decision patterns — the ONE place the metric's boundaries are defined. | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
| # cn_marker_pattern — matches any body authored by our automation. The known | ||
| # markers are: pr-review agent verdicts (<!-- pr-review-agent … -->), every | ||
| # dev-lead comment family (fix-reviews / fix-ci / noop-guard / issue / rate-limit, | ||
| # all prefixed <!-- dev-lead… -->), persona advisories (<!-- persona:… -->), and | ||
| # the dependency advisory (<!-- dependency-advisory -->). | ||
| cn_marker_pattern() { | ||
| printf '%s' '<!-- (pr-review-agent|dev-lead|persona:|dependency-advisory)' | ||
|
don-petry marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| # cn_no_action_pattern — matches a comment that asks nothing of a human. Two kinds | ||
| # of signal, both stable and grep/jq-identical (literal phrases + marker fields): | ||
| # * Known no-action bodies: | ||
| # "No actionable items found." (post_no_changes, dev-lead-fix-reviews) | ||
| # "Engine ran but made no changes." (dev-lead-fix-ci / on-mention no-changes) | ||
| # "No action required." (dependency advisory, all-LOW verdict) | ||
| # * Marker fields that are terminal no-ops: | ||
| # status=no-changes (dev-lead fix-ci / fix-reviews) | ||
| # decision=approved (pr-review clean/repeat approval) | ||
| cn_no_action_pattern() { | ||
| printf '%s' 'No actionable items found\.|Engine ran but made no changes\.|No action required\.|status=no-changes|decision=approved' | ||
|
don-petry marked this conversation as resolved.
Outdated
|
||
| } | ||
|
qodo-code-review[bot] marked this conversation as resolved.
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Pure bash classifiers (unit-tested against every known shape). | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
| # cn_is_agent_comment <body> — exit 0 when the body carries an automation marker. | ||
| cn_is_agent_comment() { | ||
| printf '%s' "${1:-}" | grep -Eq "$(cn_marker_pattern)" | ||
| } | ||
|
|
||
| # cn_is_no_action <body> — exit 0 when the body matches a no-action shape. | ||
| cn_is_no_action() { | ||
| printf '%s' "${1:-}" | grep -Eq "$(cn_no_action_pattern)" | ||
| } | ||
|
|
||
| # cn_classify <body> — prints exactly one of: non-agent | no-action | actionable. | ||
| # A body with no automation marker is "non-agent" and is never counted as noise. | ||
| cn_classify() { | ||
| local body="${1:-}" | ||
| if ! cn_is_agent_comment "$body"; then | ||
| printf 'non-agent' | ||
| return 0 | ||
| fi | ||
| if cn_is_no_action "$body"; then | ||
| printf 'no-action' | ||
| else | ||
| printf 'actionable' | ||
| fi | ||
| } | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Shared jq pre-classifier — the report's collection path applies THIS program | ||
| # (not the bash functions, for bulk speed) but keyed on the SAME patterns, passed | ||
| # in as --arg mark / --arg noact, so collection and tests agree by construction. | ||
| # --------------------------------------------------------------------------- | ||
| # Input: one GraphQL PR node (same shape reviewer_report.sh collects: .url plus | ||
| # .reviews.nodes[], .comments.nodes[], .reviewThreads.nodes[].comments.nodes[], | ||
| # each carrying .bodyText). Output: one agent_comment record per marker-bearing | ||
| # body, pre-flagged no_action. Args: --arg repo, --arg mark, --arg noact. | ||
| # shellcheck disable=SC2034 # consumed by callers via jq, not executed here | ||
| CN_AGENT_COMMENT_JQ=' | ||
| . as $pr | ||
| | ( [ (.reviews.nodes // [])[] | .bodyText ] | ||
| + [ (.comments.nodes // [])[] | .bodyText ] | ||
| + [ (.reviewThreads.nodes // [])[] | (.comments.nodes // [])[] | .bodyText ] ) | ||
| | .[] | ||
| | select(. != null) | ||
| | select(test($mark)) | ||
| | { kind: "agent_comment", repo: $repo, pr: ($pr.url), no_action: (test($noact)) } | ||
| ' | ||
|
don-petry marked this conversation as resolved.
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Pure markdown renderer over {kind:"agent_comment", pr, no_action} JSONL. | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
| # _cn_pct <numerator> <denominator> — integer percent, "n/a" when denom is 0. | ||
| _cn_pct() { | ||
| awk -v a="${1:-0}" -v b="${2:-0}" 'BEGIN { if (b <= 0) print "n/a"; else printf "%d%%", (a / b * 100) + 0.5 }' | ||
| } | ||
|
|
||
| # _cn_ratio <numerator> <denominator> — 2-decimal ratio, "0.00" when denom is 0. | ||
| _cn_ratio() { | ||
| awk -v a="${1:-0}" -v b="${2:-0}" 'BEGIN { if (b <= 0) print "0.00"; else printf "%.2f", a / b }' | ||
| } | ||
|
|
||
| # cn_render_noise_section <jsonl_dir> — read every *.jsonl in the dir, keep only | ||
| # agent_comment records, and render the "Agent comment noise" markdown section. | ||
| # PURE: no network. A dir with no agent comments renders a clean zero-state. | ||
| cn_render_noise_section() { | ||
| local dir="${1:-}" | ||
| local total=0 noact=0 prs=0 | ||
| local files=("$dir"/*.jsonl) | ||
| if [ -e "${files[0]}" ]; then | ||
| local counts | ||
| # One jq pass: total agent comments, no-action count, and distinct PRs touched. | ||
| counts="$(jq -rs ' | ||
| map(select(.kind == "agent_comment")) as $ac | ||
| | [ ($ac | length), | ||
| ($ac | map(select(.no_action)) | length), | ||
| ($ac | map(.pr) | unique | length) ] | ||
| | @tsv' "${files[@]}" 2>/dev/null || printf '0\t0\t0')" | ||
| IFS=$'\t' read -r total noact prs <<<"$counts" | ||
| fi | ||
|
don-petry marked this conversation as resolved.
|
||
| total="${total:-0}"; noact="${noact:-0}"; prs="${prs:-0}" | ||
|
|
||
| printf '## Agent comment noise\n\n' | ||
| printf -- '- **Agent comments:** %s\n' "$total" | ||
| printf -- '- **No-action comments:** %s (%s of agent comments)\n' "$noact" "$(_cn_pct "$noact" "$total")" | ||
| printf -- '- **PRs with agent comments:** %s\n' "$prs" | ||
| printf -- '- **No-action comments per PR:** %s\n\n' "$(_cn_ratio "$noact" "$prs")" | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| printf -- '- **Noise** = agent comments that ask nothing of a human — a reported no-change, a no-op engine run, or a clean/repeat approval. ' | ||
| printf 'Actionable agent comments (applied fixes, escalated findings, human-attention flags) are excluded. ' | ||
| printf 'Classification is deterministic (`scripts/lib/comment-noise.sh`), computed from the automation markers each comment already carries — no LLM.\n\n' | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.