Give the greenlight reviewer a review time budget

[jeanschmidt]

Impact: greenlight PR reviewer workflow only (no runtime/service code)
Risk: low

What

Adds a PostToolUse hook (budget-reminder.sh) that pushes "you have N minutes left" nudges into the reviewer's context, escalating through four tiers (20m target, 25m soft, 33m hard). The workflow computes the deadlines as absolute epochs in the step right before the model step; the skill gains a Time budget section telling the model how to react, including what to do when the time is spent: NO_LAND if a question critical under What to inspect is still open (a criterion never examined counts as one), LAND if only nits remain.

Why

The reviewer runs with --allowedTools Read,Glob,Grep,Write — no Bash, so it has no clock and no way to sense elapsed time. Left unpaced it explores ./pytorch until the model step's 37-minute timeout kills it, which produces no verdict at all: the run records FAILED and the scan re-dispatches the same PR, forever. A pushed reminder is the only channel that can reach the model, so the pacing lives in a hook and the skill only describes the reaction.

Notes

• The hook is advisory: PostToolUse cannot block a tool call and every path exits 0, so a fault degrades pacing, never the verdict or the write sandbox. Silence means no reminder.
• Budgets are declared once in the workflow's job env; the skill and README restate them in prose, and test_budget_reminder.py has drift guards so changing a budget in one place fails loudly instead of desyncing (it also enforces headroom below the 37m step timeout so the agent always has turns left to write the verdict).
• Tier 1 stays silent for the first half of the target window, so a review that finishes quickly never sees a reminder at all.
• Reminder intervals (180s / 60s urgent) and the verdict path are overridable by env var but are not set by the workflow — the defaults apply in production.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated
torchci	Ignored		Sep 1, 2026 6:19pm UTC

[huydhn]

We want to keep track of this somewhere in a form of metrics. If, for whatever reason, the review takes longer to run, i.e. a new model, it could effectively disable Greenlight as everything would be NO_LAND

[jeanschmidt]

agree

[jeanschmidt]

I am preparing a dashboard for greenlight, we can add this metric there. A simple reviews with NO_LAND with time >33mins

[huydhn]

Unrelated to this change, but this bubbles up on my review of this PR:

/Users/huydo/Storage/github/test-infra/greenlight/src/greenlight/decision.py:60
    if latest_status in RETRY_STATUSES:
        if _aged_out(now, latest_version, timeout):
            return Outcome(Decision.DISPATCH, "retry")
        return Outcome(Decision.WAIT, "retry_backoff")

Line 60-63, retry statuses (CANCELLED / FAILED): the only question asked is "is this row older than timeout?" If yes → DISPATCH again. There's no "how many times have we already tried this?" — nothing in decide(), and no counter in the state row it reads from.

So a review that times out writes FAILED, waits out the 45-minute window (DEFAULT_TIMEOUT_MINUTES in greenlight/src/greenlight/constants.py), and gets dispatched again — same PR, same fingerprint, same likely outcome.

The cycle

┌──────────────────────────────┬────────────────────────────────────────────┬────────────────────────────────────────────────────────┐
│             Time             │                 State row                  │                     Dr. CI section                     │
├──────────────────────────────┼────────────────────────────────────────────┼────────────────────────────────────────────────────────┤
│ t=0                          │ AI_REVIEW_DISPATCHED → AI_REVIEW_STARTED   │ "Green Light review in progress"                       │
├──────────────────────────────┼────────────────────────────────────────────┼────────────────────────────────────────────────────────┤
│ t≈40m                        │ FAILED (model step hit its 37-min timeout) │ "Green Light review did not complete" — reason: failed │
├──────────────────────────────┼────────────────────────────────────────────┼────────────────────────────────────────────────────────┤
│ t≈85m                        │ dispatched again (45-min backoff aged out) │ back to "in progress"                                  │
├──────────────────────────────┼────────────────────────────────────────────┼────────────────────────────────────────────────────────┤
│ …                            │ repeats every ~85 min                      │ flaps between those two                                │
├──────────────────────────────┼────────────────────────────────────────────┼────────────────────────────────────────────────────────┤
│ after ~24h of no PR activity │ last row stays FAILED                      │ stuck on "did not complete"                            │
└──────────────────────────────┴────────────────────────────────────────────┴────────────────────────────────────────────────────────┘

The PR never reaches LAND or NO_LAND.

Wdyt? Is this a legit scenario?

[huydhn]

Where do you get this numbers from? I try to get an understanding on why we come up with these figures. IMO, they are more important than they look because setting a wrong value here effectively disable GreenLight as we are nudging it to give up and return NO_LAND when the time run out. It's a subtle tweak IMO.

[huydhn]

When I ask this question, the agent suggests a shadow mode if we are not sure which numbers to use:

Run it in shadow first — emit the reminders, log the tier the verdict was written in, but don't yet tell the model that an unexamined criterion means NO_LAND. Then compare LAND rates.

Something to consider. On the other hand, if the budget are ample, maybe we could just go with this and see how it goes.

[jeanschmidt]

Numbers comes from: https://fburl.com/gdoc/h8wkrx0a

[jeanschmidt]

Wdyt? Is this a legit scenario?

This is exactly one of the pain points what this PR is trying to solve, tomorrow I'll be showing data on this.