Skip to content

Commit 00457e8

Browse files
committed
Wire greenlight scan to dispatch AI review
- review.run() now fingerprints each trusted-author PR, reads its latest misc.greenlight_pr_state row, and dispatches greenlight-pr-review.yml for new/changed PRs via new state.py, decision.py, and dispatch.py modules - Add scan flags --pr, --max, --ref, --timeout-minutes to the review CLI, bound into review.run through functools.partial - Extract shared statuses, key prefix, hash validators, and dispatch targets into constants.py; verdict.py consumes them and gains AI_REVIEW_STARTED - Add announce_start job to the reviewer workflow that emits the AI_REVIEW_STARTED in-flight marker to S3 before the model runs - Add head_sha to OpenPR and github_client.fingerprint_pr; update docs and add tests for the new modules and scan behavior State is re-read from ClickHouse every scan so the one-shot and --loop paths behave identically. The decide() verdict keys on eval_hash equality (clock-free) except for the in-flight and retry age windows: an AI_REVIEW_STARTED marker is left alone until --timeout-minutes (default 30) elapses, then re-dispatched. Dispatch relies on the workflow's per-PR concurrency group to supersede a still-running review rather than polling or cancelling. A per-PR fingerprint failure is isolated and logged but the scan still raises in aggregate so cron/daemon backoff sees the failure. Only the land-time verifier that reads the recorded state back remains unbuilt. Signed-off-by: Jean Schmidt <contato@jschmidt.me>
1 parent 80aad96 commit 00457e8

21 files changed

Lines changed: 1655 additions & 150 deletions

.github/workflows/greenlight-pr-review.yml

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
name: Green Light PR Review
22

3-
# Two-job privilege split for reviewing a pytorch/pytorch PR and recording a verdict:
3+
# Three jobs for reviewing a pytorch/pytorch PR and recording a verdict. review and
4+
# record split privileges so untrusted PR content never shares a job with the App key:
5+
# announce_start — emits the AI_REVIEW_STARTED in-flight marker at run start so the
6+
# scanner does not re-dispatch a PR whose review is running. No App token,
7+
# no model; S3 write only, via OIDC role/arc.
48
# review — UNPRIVILEGED. Runs the AI model over untrusted PR content. Has Bedrock
59
# (OIDC) access only; it can read code and emit a verdict file, nothing more.
610
# record — PRIVILEGED. No model. Validates the verdict, mints the Green Light App
@@ -31,6 +35,52 @@ concurrency:
3135
cancel-in-progress: true
3236

3337
jobs:
38+
announce_start:
39+
environment: greenlight-record
40+
runs-on: ubuntu-latest
41+
timeout-minutes: 15
42+
permissions:
43+
contents: read
44+
id-token: write
45+
env:
46+
PR_NUMBER: ${{ github.event.inputs.pr_number }}
47+
HEAD_SHA: ${{ github.event.inputs.head_sha }}
48+
EVAL_HASH: ${{ github.event.inputs.eval_hash }}
49+
steps:
50+
- name: Checkout test-infra
51+
uses: actions/checkout@v4
52+
with:
53+
fetch-depth: 1
54+
55+
- name: Set up Python 3.14
56+
uses: actions/setup-python@v5
57+
with:
58+
python-version: "3.14"
59+
60+
- name: Install uv
61+
uses: ./.github/actions/setup-uv
62+
63+
- name: Configure AWS credentials via OIDC
64+
uses: aws-actions/configure-aws-credentials@v4
65+
with:
66+
role-to-assume: arn:aws:iam::308535385114:role/arc
67+
aws-region: us-east-1
68+
69+
- name: Emit AI_REVIEW_STARTED marker
70+
run: |
71+
set -euo pipefail
72+
uv run --project greenlight greenlight verdict \
73+
--status AI_REVIEW_STARTED \
74+
--repo pytorch/pytorch \
75+
--pr "$PR_NUMBER" \
76+
--head-sha "$HEAD_SHA" \
77+
--eval-hash "$EVAL_HASH"
78+
79+
- name: Upload verdict row to S3
80+
run: |
81+
set -euo pipefail
82+
aws s3 cp /tmp/greenlight-verdict-row.json.gz "s3://gha-artifacts/$(cat /tmp/greenlight-verdict-key.txt)"
83+
3484
review:
3585
environment: bedrock
3686
runs-on: ubuntu-latest

greenlight/CHEATSHEET.md

Lines changed: 58 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,12 @@ directory. `just` is the front-end for every workflow — run `just` or
77
PyTorch Green Light runs one iteration of its `review` phase and exits (cron-like), or
88
loops as a daemon with `--loop`. It also has a one-shot `verdict` subcommand:
99

10-
- `review` — fetch the open PRs from a fixed set of trusted authors in `pytorch/pytorch`
11-
and log them (needs `PYTORCH_GREENLIGHT_GITHUB_TOKEN`).
10+
- `review` — scan the open PRs from a fixed set of trusted authors in `pytorch/pytorch`;
11+
for each, compute its fingerprint (`eval_hash`), read its latest state from
12+
`misc.greenlight_pr_state`, and dispatch the reviewer workflow
13+
(`greenlight-pr-review.yml` on `pytorch/test-infra`) for new or changed PRs. Needs
14+
`PYTORCH_GREENLIGHT_GITHUB_TOKEN`; any scan with at least one PR also reads ClickHouse
15+
(`CLICKHOUSE_*`).
1216
- `verdict` — record a PR-review verdict to `misc.greenlight_pr_state` (storing the
1317
passed-in `eval_hash` verbatim) and, for `LAND`/`NO_LAND`, act on the PR (approve, or
1418
dismiss greenlight's prior approval and comment). Runs once, never as a daemon.
@@ -31,18 +35,30 @@ pytest, yamllint) into `.venv`.
3135
## Run
3236

3337
```bash
34-
just review # one review iteration, then exit
35-
just run <args> # pass arbitrary args to the greenlight CLI (review is a shortcut)
38+
just review # one scan + dispatch iteration, then exit
39+
just run <args> # pass arbitrary args to the greenlight CLI (review is a shortcut)
40+
just review --pr 123 # restrict the scan to PR #123
41+
just review --max 5 # cap this iteration at 5 dispatches
42+
just review --ref my-branch # dispatch the reviewer workflow at this test-infra ref (default main)
43+
just review --timeout-minutes 60 # re-dispatch an in-flight review after 60 min (default 30)
3644
```
3745

38-
`just review` logs `INFO greenlight.review reviewing open PRs from trusted authors in
39-
pytorch/pytorch`, then queries GitHub for the trusted authors' open PRs and logs each
40-
one; without `PYTORCH_GREENLIGHT_GITHUB_TOKEN` it raises and exits `1`. Log lines are
41-
`TIMESTAMP LEVEL logger message`. Exit codes: `0` ok, `1` the phase raised, `3`
42-
another instance holds the lock (`2` is an argparse usage error).
43-
44-
`review` fetches and logs the trusted authors' open PRs but does not yet score risk or
45-
decide reviews. Risk-scoring, the AI code-review workflow, and approve/reject are planned.
46+
`just review` scans the trusted authors' open PRs and, for each PR that is new or changed
47+
since its last recorded state, dispatches the reviewer workflow
48+
(`greenlight-pr-review.yml` on `pytorch/test-infra`); an in-flight review (marked
49+
`AI_REVIEW_STARTED`) is left alone until the `--timeout-minutes` window elapses. Without
50+
`PYTORCH_GREENLIGHT_GITHUB_TOKEN` it raises and exits `1`; a scan that finds any PR also
51+
reads ClickHouse, so the `CLICKHOUSE_*` credentials must be set. Log lines are `TIMESTAMP
52+
LEVEL logger message`. Exit codes: `0` ok, `1` the phase raised, `3` another instance
53+
holds the lock (`2` is an argparse usage error).
54+
55+
The scan flags combine: `--pr N` restricts the scan to one PR, `--max N` caps how many
56+
dispatches a single iteration issues, `--ref` sets the `pytorch/test-infra` ref the
57+
reviewer workflow is dispatched at (default `main`), and `--timeout-minutes` (default 30)
58+
is how long an `AI_REVIEW_STARTED` review counts as in-flight before it is re-dispatched.
59+
That 30 is below the reviewer workflow's own ~45-55 min budget, so with the default the
60+
scanner can re-dispatch (cancel and restart) a review that is still running, and a very
61+
slow PR can loop; raise `--timeout-minutes` in the deployment if that matters.
4662

4763
Daemon mode loops the phase on an interval:
4864

@@ -56,12 +72,13 @@ runs each iteration, and on SIGTERM/SIGINT stops cleanly after the current
5672
iteration (`INFO greenlight.runner daemon stopped`, exit `0`). Signals are observed
5773
only between iterations.
5874

59-
Config comes from `PYTORCH_GREENLIGHT_*` env vars; CLI flags `--interval`, `--log-level`, and
60-
`--lock-path` override them.
75+
Config comes from `PYTORCH_GREENLIGHT_*` env vars; CLI flags `--interval`, `--log-level`,
76+
and `--lock-path` override the matching env vars, and `review` adds the scan flags `--pr`,
77+
`--max`, `--ref`, and `--timeout-minutes`.
6178

6279
| Variable | Default | Purpose |
6380
| --- | --- | --- |
64-
| `PYTORCH_GREENLIGHT_GITHUB_TOKEN` | unset | GitHub token for read-only PR access; required by `review` |
81+
| `PYTORCH_GREENLIGHT_GITHUB_TOKEN` | unset | GitHub token; `review` needs Actions: write (`workflow_dispatch`) on `pytorch/test-infra` plus PR read on `pytorch/pytorch` |
6582
| `PYTORCH_GREENLIGHT_INTERVAL_SECONDS` | `60` | Seconds between iterations in `--loop` mode |
6683
| `PYTORCH_GREENLIGHT_LOG_LEVEL` | `INFO` | Logging level (`INFO`, `DEBUG`, ...) |
6784
| `PYTORCH_GREENLIGHT_LOCK_PATH` | unset | Single-instance lock file (unset = no lock) |
@@ -74,21 +91,29 @@ logs the resolved `Config`.
7491

7592
## Simulate a run
7693

77-
The intended end-to-end flow is: `review` (fetch PRs, score risk, decide which need
78-
review) -> the AI code-review workflow (which approves or rejects).
94+
The end-to-end flow, per trusted-author PR:
95+
96+
1. `review` scans the open PRs, and for each computes its fingerprint (`eval_hash`).
97+
2. It reads the PR's latest recorded state from `misc.greenlight_pr_state`.
98+
3. If the PR is new, or its fingerprint changed since that state, and no review is
99+
in-flight within the `--timeout-minutes` window, it dispatches the reviewer workflow
100+
(`greenlight-pr-review.yml` on `pytorch/test-infra`).
101+
4. The reviewer workflow's `announce_start` job records an `AI_REVIEW_STARTED` marker, so
102+
the next scan sees the review as in-flight and does not re-dispatch it.
103+
5. The workflow reviews the PR and records its verdict through `verdict`, which emits a
104+
row to `s3://gha-artifacts/greenlight_pr_state/` (ingested into
105+
`misc.greenlight_pr_state`) and, for `LAND`/`NO_LAND`, approves or
106+
dismisses-and-comments on the PR.
79107

80-
Reality today: `review` fetches and logs the trusted authors' open PRs (a live,
81-
read-only GitHub call needing `PYTORCH_GREENLIGHT_GITHUB_TOKEN`) but does not yet score
82-
risk or decide reviews; the AI code-review workflow is not implemented. So a local run
83-
exercises the entry point and its wiring, not the real scoring, review, or
84-
approve/reject behavior.
108+
Only the land-time verifier — the pytorchbot side that reads the recorded state back at
109+
land time — is not built yet.
85110

86-
What you can run today (DEBUG to watch the flow):
111+
A scan is live: it makes real GitHub and ClickHouse calls and will really dispatch the
112+
reviewer workflow. Scope a trial run with `--pr N` and cap it with `--max N`; add
113+
`--log-level DEBUG` to watch the flow:
87114

88115
```bash
89-
just review --log-level DEBUG # fetch + log trusted authors' open PRs (needs token; no risk-scoring yet)
90-
# risk-scoring — not implemented yet
91-
# AI code-review workflow (approve/reject) — separate component, not implemented yet
116+
just review --pr 123 --max 1 --log-level DEBUG # scan one PR, dispatch at most once
92117
```
93118

94119
## Quality gates
@@ -135,9 +160,10 @@ automated DDL), so @clee2000 or @huydhn apply them manually:
135160

136161
The `verdict` subcommand does NOT write ClickHouse directly: it emits a gzipped JSON row
137162
that the record workflow uploads to `s3://gha-artifacts/greenlight_pr_state/`, and the
138-
clickhouse-replicator-s3 path ingests it into the table. greenlight keeps ClickHouse READ
139-
access for the service's SELECTs via `clickhouse_client.connect()`, which reads the standard
140-
`CLICKHOUSE_*` connection variables (`CLICKHOUSE_HOST` or its `CLICKHOUSE_ENDPOINT` alias,
141-
`CLICKHOUSE_USERNAME`, `CLICKHOUSE_PASSWORD`, and `CLICKHOUSE_PORT` default `8443`). The
142-
review-side fingerprint computation and the land-time verifier that reads this table back
143-
are not built yet.
163+
clickhouse-replicator-s3 path ingests it into the table. greenlight reads ClickHouse via
164+
`clickhouse_client.connect()` — the `review` scan looks up each PR's latest state here, by
165+
`(repo, pr_number)` — using the standard `CLICKHOUSE_*` connection variables
166+
(`CLICKHOUSE_HOST` or its `CLICKHOUSE_ENDPOINT` alias, `CLICKHOUSE_USERNAME`,
167+
`CLICKHOUSE_PASSWORD`, and `CLICKHOUSE_PORT` default `8443`). The review-side fingerprint
168+
computation is wired; only the land-time verifier that reads this table back at land time
169+
is not built yet.

greenlight/CLAUDE.md

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,21 +44,24 @@ defer; they block CI.
4444
- Single source of truth: define each value and type exactly once, import elsewhere.
4545
- No `print()` — use the logging module.
4646

47-
## The Service Seam
47+
## The Service Phase
4848

49-
PyTorch Green Light has one unit of work — the `review` phase — the placeholder seam where
50-
new logic goes:
49+
PyTorch Green Light has one unit of work — the `review` phase:
5150

52-
- `review.run()` — fetches the open PRs from a fixed set of trusted authors in
53-
`pytorch/pytorch` and logs them; the trusted-author set is the match rule. The seam
54-
to fill is risk-scoring and triggering the AI code-review workflow. Requires
55-
`PYTORCH_GREENLIGHT_GITHUB_TOKEN`.
51+
- `review.run()` — scans the open PRs from a fixed set of trusted authors in
52+
`pytorch/pytorch` (the trusted-author set is the match rule); for each PR it computes the
53+
fingerprint (`eval_hash`), reads the PR's latest state from `misc.greenlight_pr_state`,
54+
and dispatches the reviewer workflow (`greenlight-pr-review.yml` on `pytorch/test-infra`)
55+
for new or changed PRs. Requires `PYTORCH_GREENLIGHT_GITHUB_TOKEN` and `CLICKHOUSE_*`
56+
read access.
5657

57-
Approving or rejecting a PR now lives in that future triggered workflow, not in greenlight.
58+
Approving or rejecting a PR lives in the dispatched reviewer workflow (through `verdict`),
59+
not in the `review` scan itself.
5860

5961
The `eval_hash` land-guard (`pr_hash.compute_pr_hash` /
60-
`github_client.build_pr_fingerprint`) is built and tested but not yet wired into
61-
`review` or any ClickHouse table.
62+
`github_client.build_pr_fingerprint`) is computed by the `review` scan and recorded to
63+
`misc.greenlight_pr_state` by `verdict`; only the land-time verifier that reads it back at
64+
land time is not built yet.
6265

6366
Add new logic inside `review.run()`, which **must keep raising on
6467
failure** (it does not catch). The CLI runs the `review` phase

0 commit comments

Comments
 (0)