Skip to content

Commit 573f324

Browse files
jeanschmidtjeanschmidt
committed
Add pytorch-advisor-coverage lambda to classify unclassified trunk reds
**Impact:** CI observability tooling — new standalone lambda + one non-blocking build leg in the shared lambda release workflow. No effect on existing lambdas or the autorevert path. **Risk:** low ## What A new standalone AWS Lambda (`aws/lambda/pytorch-advisor-coverage`) that finds the trunk reds `/flaky_trunk` labels **unclassified** (isolated, non-persistent reds with no attaching advisor verdict — ~40% of trunk reds) and dispatches the existing AI advisor workflow on them, keyed to the observed trunk commit. Runs on an EventBridge cron (ongoing) and supports a resumable historical backfill. ## Why Roughly 40% of trunk reds never get a verdict: autorevert only evaluates its own signal subset, and the persistence heuristic can't tell an infra flake from a real regression on an isolated red. Filling those decisions gives us the data to understand why jobs/runners are flaky, without touching the revert path. # Notes - **Never triggers reverts.** Every coverage verdict is written with a hard-coded `coverage_` `signal_key` prefix, which keeps it out of autorevert's exact-match read-back (same mechanism the Dr.CI path already uses). A dispatch-time guard refuses to POST any key that isn't safely prefixed. The prefix is a module constant, not env/event-configurable, and must stay equal to torchci's `COVERAGE_PREFIX`. - **Read/dispatch only.** Reads ClickHouse (enumeration + windowless dedup), HEAD-checks S3 logs, and POSTs `workflow_dispatch`. Writes nothing to ClickHouse or S3. The minted GitHub installation token is scoped to `actions:write` only — no push/revert capability. - **Safe to ship.** `DRY_RUN=true` is the deploy default (logs intended dispatches, never POSTs); throttled by a per-run dispatch cap + inter-dispatch gap clamped under the Lambda timeout. The build leg is `continue-on-error` so a coverage build failure can't block the shared lambda release. - **Deferred / not yet live:** `/flaky_trunk` SQL doesn't yet strip the `coverage_` prefix, so coverage verdicts are written but not yet displayed there. Deploy plumbing (terraform + EventBridge + IAM) lives in `pytorch-gha-infra`; this PR only ships the lambda + its release build leg. - The Makefile vendors three self-contained `pytorch_auto_revert` helper modules at build time rather than reusing the whole CLI; keep the inlined secret/logging setup in `bootstrap.py` in sync with the sibling autorevert lambda. Signed-off-by: Jean Schmidt <contato@jschmidt.me>
1 parent f24ded7 commit 573f324

23 files changed

Lines changed: 2610 additions & 0 deletions

‎.github/workflows/_lambda-do-release-runners.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ jobs:
9090
{ dir-name: 'oss_ci_cur', zip-name: 'oss-ci-cur' },
9191
{ dir-name: 'benchmark-results-uploader', zip-name: 'benchmark-results-uploader' },
9292
{ dir-name: 'pytorch-auto-revert', zip-name: 'pytorch-auto-revert' },
93+
{ dir-name: 'pytorch-advisor-coverage', zip-name: 'pytorch-advisor-coverage' },
9394
{ dir-name: 'keep-going-call-log-classifier', zip-name: 'keep-going-call-log-classifier' },
9495
{ dir-name: 'buildkite-webhook-handler', zip-name: 'buildkite-webhook-handler' },
9596
{ dir-name: 'benchmark_regression_summary_report', zip-name: 'benchmark-regression-summary-report' },
@@ -98,6 +99,11 @@ jobs:
9899
]
99100
name: Build ${{ matrix.dir-name }} lambda
100101
runs-on: ubuntu-latest
102+
# The advisor-coverage leg is non-blocking: a coverage build failure must not
103+
# fail this matrix job (fail-fast is already off) and block publishing the
104+
# shared lambda release (pytorch-auto-revert et al.). A failed continue-on-error
105+
# leg reports success to `release`'s `needs`, and simply omits its zip.
106+
continue-on-error: ${{ matrix.dir-name == 'pytorch-advisor-coverage' }}
101107
permissions:
102108
contents: read
103109
steps:

‎aws/lambda/pytorch-advisor-coverage/.env.example‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# ClickHouse Configuration (read-only)
2+
CLICKHOUSE_HOST=your_clickhouse_host
3+
CLICKHOUSE_PORT=8443
4+
CLICKHOUSE_USERNAME=default
5+
CLICKHOUSE_PASSWORD=your_password
6+
CLICKHOUSE_DATABASE=default
7+
8+
# GitHub App (PEM fetched from Secrets Manager in prod via SECRET_STORE_NAME).
9+
# For local dev you may instead set a raw GITHUB_TOKEN.
10+
GITHUB_APP_ID=
11+
GITHUB_INSTALLATION_ID=
12+
GITHUB_APP_SECRET=
13+
GITHUB_TOKEN=
14+
SECRET_STORE_NAME=
15+
16+
# Coverage core
17+
REPO_FULL_NAME=pytorch/pytorch
18+
# Empty = all trunk workflows; set a JSON array or CSV to filter.
19+
WORKFLOWS=
20+
HOURS=24
21+
# Min total runs for a job to be enumerated (matches the /flaky_trunk page).
22+
MIN_RUNS=20
23+
24+
# Mode: ongoing (enumerate [now-HOURS, now)) | backfill (chunked time range)
25+
MODE=ongoing
26+
AS_OF_START=
27+
AS_OF_END=
28+
AS_OF_STEP_HOURS=24
29+
30+
# Throttle
31+
MAX_DISPATCHES_PER_RUN=10
32+
DISPATCH_GAP_SECONDS=3
33+
34+
# Safety: DRY_RUN=true logs intended dispatches without POSTing. Set to false to
35+
# arm real dispatch. The coverage_ signal_key prefix is NOT configurable.
36+
DRY_RUN=true
37+
LOG_LEVEL=INFO

‎aws/lambda/pytorch-advisor-coverage/.gitignore‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
*.zip
2+
deployment/
3+
venv/
4+
*.html

‎aws/lambda/pytorch-advisor-coverage/Makefile‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
all: test
2+
3+
.PHONY: clean
4+
clean:
5+
rm -rf deployment
6+
rm -rf deployment.zip
7+
rm -rf venv
8+
# it makes sense for this to be the last one, or at least after rm -rf venv
9+
find . -name __pycache__ -type d | xargs rm -rf
10+
11+
venv/bin/python:
12+
virtualenv venv
13+
venv/bin/pip install -r requirements.txt
14+
venv/bin/pip install -r dev_requirements.txt
15+
16+
deployment.zip:
17+
mkdir -p deployment
18+
cp -a advisor_coverage ./deployment/.
19+
# Vendor only the self-contained pytorch_auto_revert helpers still imported
20+
# at runtime (client factories, dispatch primitive, RetryWithBackoff,
21+
# parse_datetime). These three modules have no intra-package imports, so we
22+
# copy just them (+ the package __init__) rather than the whole CLI. Copied
23+
# fresh at build time so the vendored source stays pristine.
24+
mkdir -p ./deployment/pytorch_auto_revert
25+
cp -a ../pytorch-auto-revert/pytorch_auto_revert/__init__.py ./deployment/pytorch_auto_revert/.
26+
cp -a ../pytorch-auto-revert/pytorch_auto_revert/utils.py ./deployment/pytorch_auto_revert/.
27+
cp -a ../pytorch-auto-revert/pytorch_auto_revert/clickhouse_client_helper.py ./deployment/pytorch_auto_revert/.
28+
cp -a ../pytorch-auto-revert/pytorch_auto_revert/github_client_helper.py ./deployment/pytorch_auto_revert/.
29+
cp -a __init__.py ./deployment/.
30+
pip3.10 install -r requirements.txt -t ./deployment/. --platform manylinux2014_x86_64 --only-binary=:all: --implementation cp --python-version 3.10 --upgrade
31+
cd ./deployment && zip -q -r ../deployment.zip .
32+
33+
.PHONY: create-deployment-package
34+
create-deployment-package: deployment.zip
35+
36+
.PHONY: test
37+
test: venv/bin/python
38+
venv/bin/python -m pytest advisor_coverage/tests -v
39+
40+
.PHONY: lintrunner
41+
lintrunner: venv/bin/python
42+
# lintrunner only works properly with virtualenv if you activate it first
43+
. venv/bin/activate && lintrunner init --config ../../../.lintrunner.toml
44+
. venv/bin/activate && lintrunner -a -v --force-color --config ../../../.lintrunner.toml --paths-cmd='git grep -Il .'

‎aws/lambda/pytorch-advisor-coverage/README.md‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
# pytorch-advisor-coverage
2+
3+
A standalone AWS Lambda that drives down the `/flaky_trunk` page's
4+
**unclassified** bucket — the isolated, non-persistent trunk reds that have no
5+
attaching advisor verdict and that autorevert never evaluates (~40% of trunk
6+
reds). It dispatches the `claude-autorevert-advisor.yml` workflow on those reds,
7+
keyed to the OBSERVED trunk commit. The workflow writes the verdict JSON to S3;
8+
the existing `clickhouse-replicator-s3` Lambda ingests it into
9+
`misc.autorevert_advisor_verdicts`.
10+
11+
It vendors a few self-contained helpers from the sibling `pytorch_auto_revert`
12+
package (ClickHouse/GitHub client factories, the dispatch primitive,
13+
`RetryWithBackoff`, `parse_datetime`) but does NOT reuse its partition/signal
14+
logic — targeting is a direct ClickHouse enumeration.
15+
16+
## What "unclassified" means (reused verbatim from /flaky_trunk)
17+
18+
Eligibility reuses the exact category=5 definition from torchci
19+
`flaky_trunk_jobs/query.sql`: a trunk red is **unclassified** when it
20+
21+
- is red at a trunk commit, AND
22+
- has NO attaching advisor verdict (related/revert/infra_issue/not_related/garbage)
23+
keyed to that commit, AND
24+
- is NOT structurally persistent (no adjacent hard-red on the previous or next
25+
trunk commit) — i.e. an isolated single-commit hard-red, a retry-green, or a
26+
green→red→green flake.
27+
28+
The advisor classifies these well (a feasibility test caught a real regression
29+
the persistence heuristic misses).
30+
31+
## The revert-isolation invariant (load-bearing)
32+
33+
The verdicts table is a **live control input**: autorevert reads it back and a
34+
high-confidence `revert`/`related` verdict triggers a real revert. Its read-back
35+
is an EXACT `signal_key` match.
36+
37+
Every coverage verdict carries `signal_key = "coverage_" + <real job name>`. The
38+
prefix keeps it out of autorevert's extracted key set, so it is **never attached
39+
and never drives a revert or veto** — the same mechanism the PR-side Dr.CI path
40+
(`dr_ci_`) already uses.
41+
42+
- The prefix is a **hard-coded module constant** `COVERAGE_SIGNAL_KEY_PREFIX`
43+
(`config.py`), NOT env/event configurable.
44+
- It **must stay equal** to torchci `COVERAGE_PREFIX` in
45+
`torchci/lib/advisorVerdictUtils.ts`.
46+
- A dispatch-time guard refuses to POST if the outgoing key is not safely
47+
prefixed (never equal to the native key).
48+
49+
## How a red becomes a dispatch
50+
51+
1. **Enumerate** currently-unclassified trunk reds for the window (one CH query
52+
reusing the flaky_trunk CTE chain), plus a few green baseline-before commits
53+
of the same job (a second CH query).
54+
2. **Windowless dedup**: skip any red that already has a `coverage_`-prefixed
55+
verdict for `(repo, observed commit, key)` — no time filter.
56+
3. **Log-readability pre-filter**: HEAD-check the raw log at
57+
`ossci-raw-job-status/log/{job_id}` and skip stubs (<1000 bytes) or missing
58+
logs (~16% are unusable). Counted as `skipped_no_log`.
59+
4. **Build** the isolated-red `signal_pattern` (failed suspect + green baselines)
60+
and **dispatch** with `suspect_commit = observed trunk commit`, `pr_number="0"`.
61+
62+
The advisor gets the diff from the workflow's OWN checkout of `suspect_commit`,
63+
so the dispatch token needs no PR-read scope (`pr_number` is just metadata).
64+
65+
## Deferred (not yet live)
66+
67+
The `/flaky_trunk` SQL does not yet strip the `coverage_` prefix in `advisor_agg`
68+
before its job-name join. **Until that change lands, coverage verdicts are
69+
written but NOT yet displayed on /flaky_trunk** (and, usefully, they do not yet
70+
reclassify the red out of the unclassified bucket — the windowless dedup is what
71+
prevents re-dispatch in the meantime). No confidence gate is applied: any
72+
non-`unsure` verdict is a classification.
73+
74+
## What it does NOT do
75+
76+
- Writes **nothing** to ClickHouse or S3. It only READS ClickHouse, HEAD-checks
77+
S3 logs, and POSTs `workflow_dispatch`.
78+
- The minted GitHub installation token is scoped to `actions:write` only — it
79+
cannot push or revert.
80+
81+
## Ongoing vs backfill
82+
83+
- **Ongoing** (`MODE=ongoing`, the EventBridge cron): enumerate `[now - HOURS, now)`.
84+
- **Backfill** (`MODE=backfill`): tile `[AS_OF_START, AS_OF_END)` into
85+
`AS_OF_STEP_HOURS` chunks and dispatch each chunk's unclassified reds,
86+
throttled + resumable.
87+
88+
### Running a long backfill (resume cursor)
89+
90+
A single Lambda invocation is bounded by a wall-clock budget and the dispatch
91+
cap; when it stops early it returns `next_as_of`. Re-invoke with
92+
`as_of_start = next_as_of` until it is `null`:
93+
94+
```jsonc
95+
{"mode": "backfill", "as_of_start": "2026-02-19", "as_of_end": "2026-08-18"}
96+
// response -> {"next_as_of": "2026-02-20T00:00:00", ...} ; repeat until null
97+
```
98+
99+
Locally, one process completes the whole range (unlimited budget, only the gap
100+
throttle applies):
101+
102+
```bash
103+
MODE=backfill AS_OF_START=2026-02-19 AS_OF_END=2026-08-18 \
104+
python -m advisor_coverage.backfill
105+
```
106+
107+
## Kill switch
108+
109+
- Set `DRY_RUN="true"` (the deploy default): logs intended dispatches, never
110+
POSTs. Flipping to `"false"` arms real dispatch.
111+
- Or disable the EventBridge rule for the ongoing cron.
112+
113+
## Throttle
114+
115+
- `MAX_DISPATCHES_PER_RUN` (default 10) — per invocation, clamped by a compiled
116+
`HARD_CAP` (100) and the Lambda timeout budget; env/event may only LOWER it.
117+
- `DISPATCH_GAP_SECONDS` (default 3) — sleep between dispatches (floored to 1s).
118+
119+
Cross-invocation duplicates (a red re-dispatched before its verdict lands) are
120+
accepted: safe (prefixed → no reverts) and bounded by the throttle. Intra-run
121+
duplicates from overlapping windows are suppressed in memory.
122+
123+
## Known limitation
124+
125+
Backfill chunks are enumerated with a persistence lookback/lookahead margin
126+
(`PERSISTENCE_MARGIN_HOURS`) on each side, so a red at a chunk boundary still
127+
sees its neighbouring trunk commits for the lag/lead persistence check (reds are
128+
dispatched only within the core chunk). The one remaining edge is inherent: the
129+
newest trunk commit in ongoing mode has no lookahead (the next commit doesn't
130+
exist yet), so a just-observed red that will turn out persistent may be
131+
dispatched once. This is bounded, safe (non-reverting), and matches
132+
/flaky_trunk's own newest-commit semantics.
133+
134+
## Environment variables
135+
136+
| Var | Default | Notes |
137+
|---|---|---|
138+
| `CLICKHOUSE_HOST` / `CLICKHOUSE_PORT` | `localhost` / `8443` | read-only CH |
139+
| `CLICKHOUSE_USERNAME` / `CLICKHOUSE_PASSWORD` | `""` | reuse `CLICKHOUSE_USER_AUTO_REVERT`; password from Secrets Manager |
140+
| `CLICKHOUSE_DATABASE` | `default` | |
141+
| `GITHUB_APP_ID` / `GITHUB_INSTALLATION_ID` | `""` / `0` | GitHub App; PEM from Secrets Manager |
142+
| `GITHUB_APP_SECRET` / `GITHUB_TOKEN` | `""` | base64 PEM / raw token (local dev) |
143+
| `SECRET_STORE_NAME` | `""` | `pytorch-autorevert-secrets` in prod |
144+
| `REPO_FULL_NAME` | `pytorch/pytorch` | pinned to an allowlist |
145+
| `WORKFLOWS` | empty = all | optional JSON array / CSV filter |
146+
| `HOURS` | `24` | ongoing enumeration window |
147+
| `MIN_RUNS` | `20` | min total runs for a job to be enumerated (matches /flaky_trunk) |
148+
| `MODE` | `ongoing` | `ongoing` \| `backfill` |
149+
| `AS_OF_START` / `AS_OF_END` | — | backfill range (UTC) |
150+
| `AS_OF_STEP_HOURS` | `24` | backfill chunk size |
151+
| `MAX_DISPATCHES_PER_RUN` | `10` | see Throttle |
152+
| `DISPATCH_GAP_SECONDS` | `3` | see Throttle |
153+
| `DRY_RUN` | `true` | `false` arms real dispatch |
154+
| `LOG_LEVEL` | `INFO` | secret-leaking loggers pinned to WARNING regardless |
155+
156+
The `coverage_` prefix is intentionally NOT an env var.
157+
158+
## Local run + tests
159+
160+
```bash
161+
python -m advisor_coverage # ongoing dry-run (set CLICKHOUSE_* first)
162+
make test # mocked unit tests, no network
163+
```
164+
165+
## Build / deploy
166+
167+
`make deployment.zip` vendors the three needed `pytorch_auto_revert` helper
168+
modules plus the Python deps. The handler is
169+
`advisor_coverage.handler.lambda_handler`. Deploy plumbing (terraform +
170+
EventBridge cron + IAM) lives in `pytorch-gha-infra`.

‎aws/lambda/pytorch-advisor-coverage/__init__.py‎

Whitespace-only changes.

‎aws/lambda/pytorch-advisor-coverage/advisor_coverage/__init__.py‎

Whitespace-only changes.

‎aws/lambda/pytorch-advisor-coverage/advisor_coverage/__main__.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
from .handler import main_cli
2+
3+
4+
if __name__ == "__main__":
5+
main_cli()

0 commit comments

Comments
 (0)