diff --git a/scripts/fleet_monitor.sh b/scripts/fleet_monitor.sh index 01b2c4aa2..bb30d9dc3 100755 --- a/scripts/fleet_monitor.sh +++ b/scripts/fleet_monitor.sh @@ -267,6 +267,12 @@ fi # repos with >=1 marker in the window get a row; the section is omitted entirely # when the fleet had no dev-lead activity. Best-effort: a comment-read failure on # one repo is a per-repo warning, not a fatal error for the monitor. +# +# The timeout rate is reported as prevalence (timeout / total dev-lead runs, #1030), +# so each row also carries the repo's total `dev-lead.yml` runs in the window — +# read from the per-workflow metrics already collected above (matched by repo + +# workflow basename), not a new API scan. A missing row or non-numeric total (the +# ERROR sentinel `?`) sanitizes to 0, which the report renders as `n/a`. # --------------------------------------------------------------------------- dev_lead_reason_file=$(mktemp) for repo in "${repos[@]}"; do @@ -280,11 +286,33 @@ for repo in "${repos[@]}"; do IFS=$'\t' read -r dl_timeout dl_engine_error dl_total \ < <(summarize_dev_lead_timeouts "$comments_json") if [ "${dl_total:-0}" -gt 0 ]; then - printf '%s\t%s\t%s\t%s\n' "$repo" "$dl_timeout" "$dl_engine_error" "$dl_total" \ + # Total dev-lead runs = the `dev-lead.yml` row's `total` (field 4) for this + # repo in metrics_file, matched by workflow-file basename. Empty when the + # repo has no dev-lead.yml row; '?' when that row is an ERROR sentinel. + dl_runs=$(awk -F'\t' -v r="$repo" ' + $2 == r { + n = split($3, parts, "/") + if (parts[n] == "dev-lead.yml") { print $4; exit } + }' "$metrics_file") + case "${dl_runs:-}" in + ''|*[!0-9]*) dl_runs=0 ;; + esac + printf '%s\t%s\t%s\t%s\t%s\n' \ + "$repo" "$dl_timeout" "$dl_engine_error" "$dl_total" "$dl_runs" \ >> "$dev_lead_reason_file" fi done +# Fleet-wide total dev-lead runs in the window = the sum of every repo's +# `dev-lead.yml` row `total` (field 4) in the per-workflow metrics, across ALL +# repos — including all-success repos with no markers. This is the prevalence +# denominator for the timeout rate (#1030); summing only marker repos would +# inflate the rate. Non-numeric totals (the ERROR sentinel `?`) are skipped. +fleet_dl_runs=$(awk -F'\t' ' + { n = split($3, p, "/"); if (p[n] == "dev-lead.yml" && $4 ~ /^[0-9]+$/) s += $4 } + END { print s + 0 } +' "$metrics_file") + # --------------------------------------------------------------------------- # 3. Generate reports # --------------------------------------------------------------------------- @@ -311,7 +339,7 @@ stub_drift_section() { dev_lead_timeout_section() { [ -s "$dev_lead_reason_file" ] || return 0 printf '\n' - generate_dev_lead_timeout_report "$dev_lead_reason_file" + generate_dev_lead_timeout_report "$dev_lead_reason_file" "${fleet_dl_runs:-0}" } # Step Summary — Tier 1 visualizations only (Mermaid not rendered there) diff --git a/scripts/fleet_report.sh b/scripts/fleet_report.sh index 13ec7068e..9c18eb2e0 100755 --- a/scripts/fleet_report.sh +++ b/scripts/fleet_report.sh @@ -148,40 +148,63 @@ summarize_dev_lead_timeouts() { # generate_dev_lead_timeout_report # Renders the Dev-Lead timeout ("wall") observability section (#1019, story C of -# #901). Input is a TSV file with one row per repo that had >=1 dev-lead marker -# in the window: `repo timeout engine_error total`. Prints a -# per-repo table, fleet totals, and the timeout rate (timeouts / all dev-lead -# failure markers) — the "wall rate" Story A/B aim to drive down. An empty or -# missing file prints nothing, so the section is omitted when there was no -# dev-lead activity in the window. +# #901; prevalence rate #1030). Input is a TSV file with one row per repo that +# had >=1 dev-lead marker in the window: +# `repo timeout engine_error markers total_runs` +# where `markers` is the count of all dev-lead failure/retry/escalation markers +# (the composition breakdown) and `total_runs` is the repo's total dev-lead.yml +# runs in the window (the prevalence denominator, from the fleet monitor's +# per-workflow metrics). Prints a per-repo table (raw timeout + engine-error +# counts retained), fleet totals, and the timeout **prevalence** rate +# (`reason=timeout` / total dev-lead runs — over ALL runs, not just failures) — +# the "wall rate" Story A/B aim to drive down. When total_runs is missing, +# non-numeric (e.g. an ERROR sentinel), or zero for a repo it is shown as `n/a` +# and contributes 0 to the denominator (no divide-by-zero). An empty or missing +# file prints nothing, so the section is omitted when there was no dev-lead +# activity in the window. generate_dev_lead_timeout_report() { - local f="${1:-}" + local f="${1:-}" fleet_runs="${2:-}" [ -n "$f" ] && [ -s "$f" ] || return 0 + # Prevalence denominator = ALL dev-lead runs fleet-wide (passed in by the + # caller from the per-workflow metrics), NOT just runs from repos that happened + # to have a marker. Summing only marker repos drops every all-success repo from + # the denominator and inflates the rate — the exact flaw #1030 set out to fix. + case "${fleet_runs:-}" in ''|*[!0-9]*) fleet_runs=0 ;; esac - local repo t e tot + local repo t e tot runs runs_disp local sum_t=0 sum_e=0 sum_tot=0 printf '## Dev-Lead Timeouts (walls)\n\n' printf 'Stage timeouts (`reason=timeout`) that escalate to a human, broken out from generic `engine-error`, over the window.\n\n' - printf '| Repo | Timeouts | Engine-errors | Total dev-lead failures |\n' - printf '|---|---|---|---|\n' - while IFS=$'\t' read -r repo t e tot; do + printf '| Repo | Timeouts | Engine-errors | Failure markers | Total runs |\n' + printf '|---|---|---|---|---|\n' + while IFS=$'\t' read -r repo t e tot runs; do [ -n "$repo" ] || continue - printf '| `%s` | %s | %s | %s |\n' "$repo" "$t" "$e" "$tot" + # Sanitize total_runs: empty / non-numeric (ERROR sentinel '?') → 0 → n/a. + case "${runs:-}" in + ''|*[!0-9]*) runs=0 ;; + esac + runs_disp="n/a" + [ "$runs" -gt 0 ] && runs_disp="$runs" + printf '| `%s` | %s | %s | %s | %s |\n' "$repo" "$t" "$e" "$tot" "$runs_disp" sum_t=$(( sum_t + ${t:-0} )) sum_e=$(( sum_e + ${e:-0} )) sum_tot=$(( sum_tot + ${tot:-0} )) done < "$f" - printf '| **Fleet total** | **%s** | **%s** | **%s** |\n\n' "$sum_t" "$sum_e" "$sum_tot" + local runs_total_disp="n/a" + [ "$fleet_runs" -gt 0 ] && runs_total_disp="$fleet_runs" + printf '| **Fleet total** | **%s** | **%s** | **%s** | **%s** |\n\n' \ + "$sum_t" "$sum_e" "$sum_tot" "$runs_total_disp" + printf '_Per-repo rows list only repos with >=1 dev-lead marker in the window; the **Total runs** fleet total (and the rate denominator) include **all** dev-lead runs fleet-wide, incl. all-success repos with no markers._\n\n' local rate - rate=$(awk -v t="$sum_t" -v n="$sum_tot" 'BEGIN { + rate=$(awk -v t="$sum_t" -v n="$fleet_runs" 'BEGIN { if (n <= 0) { print "n/a"; exit } pct = t * 100 / n printf (pct == int(pct)) ? "%d%%" : "%.1f%%", pct }') - printf '**Timeout rate** (`reason=timeout` / all dev-lead failure markers): %s (%s / %s)\n' \ - "$rate" "$sum_t" "$sum_tot" + printf '**Timeout rate** (`reason=timeout` / total dev-lead runs fleet-wide — prevalence over all runs, not just failures): %s (%s / %s)\n' \ + "$rate" "$sum_t" "$runs_total_disp" } # detect_systemic_failures diff --git a/tests/fleet_report.bats b/tests/fleet_report.bats index c9b8c1f62..2f05106b9 100755 --- a/tests/fleet_report.bats +++ b/tests/fleet_report.bats @@ -405,44 +405,100 @@ _mk_metrics() { } # --------------------------------------------------------------------------- -# generate_dev_lead_timeout_report — renders the per-repo section (#1019) -# Input: TSV file rows `repo timeout engine_error total`. +# generate_dev_lead_timeout_report — renders the per-repo section (#1019/#1030) +# Input: TSV file rows +# `repo timeout engine_error markers total_runs`. +# The timeout rate is prevalence: timeout / total_runs (over ALL dev-lead runs, +# not just failure markers). # --------------------------------------------------------------------------- -@test "dev-lead timeout report: renders per-repo rows, fleet total, and timeout rate" { +@test "dev-lead timeout report: renders per-repo rows, fleet total, and prevalence rate" { local f f="$(mktemp "$BATS_TEST_TMPDIR/fr.XXXXXX")" + # timeouts 2+1=3; total runs 60+40=100 → prevalence 3/100 = 3% printf '%s\n' \ - $'petry-projects/a\t2\t1\t4' \ - $'petry-projects/b\t1\t1\t3' \ + $'petry-projects/a\t2\t1\t4\t60' \ + $'petry-projects/b\t1\t1\t3\t40' \ > "$f" - run generate_dev_lead_timeout_report "$f" + run generate_dev_lead_timeout_report "$f" 100 [ "$status" -eq 0 ] [[ "$output" =~ "petry-projects/a" ]] [[ "$output" =~ "petry-projects/b" ]] - # Fleet totals: timeout 3, engine-error 2, total 7 + # Fleet totals: timeout 3, engine-error 2, markers 7 (composition breakdown retained) [[ "$output" =~ "Fleet total" ]] [[ "$output" =~ "reason=timeout" ]] - # Rate = 3 / 7 = 42.9% - [[ "$output" =~ "42.9%" ]] + # Prevalence rate = K/N = 3 / 100 = 3% (over total runs, not failures) + [[ "$output" =~ "3%" ]] + [[ "$output" =~ "3 / 100" ]] + # Not the old composition rate (3 / 7 = 42.9%) + [[ ! "$output" =~ "42.9%" ]] + rm -f "$f" +} + +@test "dev-lead timeout report: denominator is ALL fleet runs, not just marker repos (#1030)" { + local f + f="$(mktemp "$BATS_TEST_TMPDIR/fr.XXXXXX")" + # Only repo a has markers (2 timeouts over its 10 runs). But the fleet ran 100 + # dev-lead runs total — repo b had 90 all-success runs (no markers, absent from + # the file). Prevalence MUST be 2/100 = 2%, not 2/10 = 20% (the marker-repo-only + # denominator bug). The passed-in fleet total is the denominator. + printf '%s\n' $'petry-projects/a\t2\t0\t2\t10' > "$f" + run generate_dev_lead_timeout_report "$f" 100 + [ "$status" -eq 0 ] + [[ "$output" =~ "2 / 100" ]] + [[ ! "$output" =~ "20%" ]] + rm -f "$f" +} + +@test "dev-lead timeout report: prevalence rate K/N renders a decimal when needed" { + local f + f="$(mktemp "$BATS_TEST_TMPDIR/fr.XXXXXX")" + # K=3 timeouts over N=8 total runs → 37.5% + printf '%s\n' $'petry-projects/a 3 0 3 8' > "$f" + run generate_dev_lead_timeout_report "$f" 8 + [ "$status" -eq 0 ] + [[ "$output" =~ "37.5%" ]] + [[ "$output" =~ "3 / 8" ]] rm -f "$f" } @test "dev-lead timeout report: whole-number rate renders without a decimal" { local f f="$(mktemp "$BATS_TEST_TMPDIR/fr.XXXXXX")" - printf '%s\n' $'petry-projects/a\t1\t1\t2' > "$f" - run generate_dev_lead_timeout_report "$f" + # 1 timeout over 2 total runs → 50% + printf '%s\n' $'petry-projects/a 1 1 2 2' > "$f" + run generate_dev_lead_timeout_report "$f" 2 [[ "$output" =~ "50%" ]] [[ ! "$output" =~ "50.0%" ]] rm -f "$f" } +@test "dev-lead timeout report: zero total runs yields n/a (no divide-by-zero)" { + local f + f="$(mktemp "$BATS_TEST_TMPDIR/fr.XXXXXX")" + printf '%s\n' $'petry-projects/a 1 0 1 0' > "$f" + run generate_dev_lead_timeout_report "$f" 0 + [ "$status" -eq 0 ] + [[ "$output" =~ "n/a" ]] + rm -f "$f" +} + +@test "dev-lead timeout report: unavailable (non-numeric) total runs is treated as n/a" { + local f + f="$(mktemp "$BATS_TEST_TMPDIR/fr.XXXXXX")" + # 5th column is the ERROR sentinel '?' — must not crash or divide-by-zero + printf '%s\n' $'petry-projects/a 1 0 1 ?' > "$f" + run generate_dev_lead_timeout_report "$f" '?' + [ "$status" -eq 0 ] + [[ "$output" =~ "n/a" ]] + rm -f "$f" +} + @test "dev-lead timeout report: empty file produces no output" { local f f="$(mktemp "$BATS_TEST_TMPDIR/fr.XXXXXX")" : > "$f" - run generate_dev_lead_timeout_report "$f" + run generate_dev_lead_timeout_report "$f" 0 [ "$status" -eq 0 ] [ -z "$output" ] rm -f "$f"