feat: weekly assignment rate limit - #197
Merged
Merged
Conversation
A pure-SQL probe over analyzer_reviewerassignmentapplication answering how
much *new* work each reviewer is actually assigned per rolling week — the
measurement the reviewer assignment rate limit (doc 054) has to be sized
against.
Twelve sections: source health, trailing 7/30-day intake per reviewer, peak
rolling 7-day window (all history and active-reviewers-only), what-if replays
of candidate limits over 90 and 30 days, concurrent cap vs actual weekly
intake, and the three sharp edges the design flags — login case, distinct-PR
vs row counting, and pull-claim provenance via a NULL snapshot_id.
Runs with no dyno and no deploy; heroku pg:psql executes it locally against
production:
heroku pg:psql -a queueboard-backend -f scripts/probe_054_rate_limit.sql
Read-only apart from one temp view it drops at the end. Reviewer logins are
pseudonymised by default so output can be pasted into the design doc; flip
`\set show_logins 0` to 1 in the file for real logins (heroku pg:psql does not
forward psql's -v).
Validated against a seeded local Postgres 16 in both login modes, and run
against production three times while iterating on the doc.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Proposes a rolling weekly cap on *new* assignments per reviewer (ReviewerPreference.max_new_assignments_per_week, opt-in/null by default), additive to maximum_capacity: the existing cap bounds stock, not flow, so it only limits reviewers who don't act. One reviewer-facing number that is both the throughput limit and the smoother; catch-up stays the pull side's job (doc 053 overrides it like every other push throttle). Decisions closed in review: window length 7 days; no ANALYZER_ASSIGNMENT_RATE_LIMIT_ENABLED flag — unlike 046/050/053 this adds no behavior of its own, the opt-in default is the off switch, and the rollback is clearing the pilot cohort's limits. Includes a Measured Baseline from scripts/probe_054_rate_limit.sql against production (mathlib4, 839 applied rows since 2026-06-23), which confirms the premise and re-sized several claims: - maximum_capacity does not bound flow: three reviewers capped at 10 concurrent took 22-30 new PRs in 30 days. - 5/week is the median active reviewer's *worst* week — a no-op for the median, binding for 13 of 32 active reviewers, withholding ~30% of their intake. Right size for an opt-in knob, wrong size for a global default. - Login case is a live defect risk: 11 of 41 reviewers are stored capitalized, so a case-sensitive count returns zero for them and their limit would silently never fire. lower() on both sides is load-bearing. - Distinct-PR counting is insurance: zero re-assignment churn in 67 days. - Single-night clustering is 1-2 PRs, not the burst Subtlety 6 tolerates. Excluding the rollout period lowers the affected headcount but *raises* the withheld share (26.9% -> 29.6%), because recent intake is more concentrated — recorded explicitly, since it is easy to get backwards. Design only; no implementation yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ReviewerPreference.maximum_capacity` caps the *stock* of PRs a reviewer holds at once, not the *flow* they take on: a reviewer who clears quickly frees the slot and the next nightly run refills it. Measured on production, reviewers with `maximum_capacity=10` took 22-30 new PRs in 30 days. Add `max_new_assignments_per_week` (nullable) as the flow bound, plus the `ANALYZER_ASSIGNMENT_RATE_WINDOW_DAYS` setting that defines what "per week" means. Null is unlimited, which is both the default and the rollout switch — this commit changes no behavior on its own. Wired through the admin changelist and the reviewer-topics.json importer and exporter; the export omits an unset limit so a pre-054 file round-trips as "no limit" rather than silently un-limiting anyone. Design doc 054. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`recent_assignment_counts` answers the one question the rate limit needs: how many distinct PRs was this reviewer newly assigned in this repo within the trailing window? Reads the existing `ReviewerAssignmentApplication` history (design doc 046), so there is no new log table and no backfill. Two counting rules earn their keep: - `lower()` on both sides. `reviewer_login` is stored verbatim — the nightly paths pass engine logins while the console accept and the 053 claim pass `User.github_login`, which keeps GitHub's casing. 11 of 41 production reviewers are stored capitalized, and since no login appears under two spellings, a case-sensitive filter would not undercount them, it would return zero and silently disable their limit. - Distinct PRs, not rows, so an auto-unassigned-then-reassigned PR counts once. Currently insurance rather than a live correction (zero churn in 67 days). Design doc 054. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ReviewerProfile` gains `weekly_limit`, `recent_assignment_count` and `simulated_this_run`; `_reviewer_candidate_state` requires a reviewer to pass both the stock gate (`maximum_capacity`) and the new flow gate. All three fields default to a no-op, because that gate is shared code — the nightly builder and 053's suggestions both call it — so every pre-054 construction site keeps today's behavior without naming them. The gate is a strict `<`: "max 5 per week" means at most 5, so a reviewer with four in the window still receives a fifth. `simulated_this_run` charges this run's own picks against the budget, without which one night could spend a whole week's allowance several times over; `run_assignment_simulation` folds each pick back into the picked reviewer's profile beside the existing weight bump. Counts and limits are injected in `build_reviewer_catalog` rather than at `prepare_assignment_inputs`. One grouped query per catalog build, and every consumer of a catalog — builder, trace, suggestions, and the reviewer-facing load line — then reads the identical figure by construction, so the number that silenced a reviewer's push is the number they are shown. The diagnostic trace records `at_rate_limit` separately from `at_capacity`: a reviewer withheld while visibly holding free capacity is otherwise unexplainable. Design doc 054. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…erride it Two halves of the same requirement: enforcement ships with its explanation. 053's override profile gains `weekly_limit=None`, joining `maximum_capacity`, `auto_assign` and `away_until` as a push throttle an explicit request ignores (053 Invariant 4). This is load-bearing rather than tidy: a weekly window deliberately gives up saving unused budget, so catch-up has to live on the pull side, and this is it. The skip tally needs no `at_rate_limit` row for the same reason it has no `at_capacity` — the requester's gates are always overridden. `ReviewerLoad` carries `weekly_count` / `weekly_limit` / `at_weekly_limit`, read off the same profile the engine gate uses, and `format_load_line` appends `· last 7 days: 4 / 5` (with `⚠ weekly limit reached` once spent). One change reaches `assigned-prs`, the daily attention DM, the console and 053 at once. Wording is "last N days", not "this week", and N comes from the setting that defines the window: it is rolling, and the calendar reading produces a "why am I blocked, it's Monday" bug report. A reviewer with no limit gets a byte-for-byte unchanged line. Design doc 054. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the field to `/console/preferences/`, alongside `maximum_capacity`. Reviewers cannot pick a number they cannot see: measured intake is a median of 2/week against a median *worst* week of 5, so the help text shows the reviewer their own trailing intake next to the input. The count is computed in the console view and passed in, because it is an `analyzer` figure and `core` does not import `analyzer`; omitting it drops the sentence and nothing else. Label and help text both name the window from `ANALYZER_ASSIGNMENT_RATE_WINDOW_DAYS`, so the copy cannot drift from the mechanism, and both say "N days" rather than "per week" — the window is rolling. Blank clears the limit. A limit of 0 is rejected: "never assign me anything" is `auto_assign` off, which says so on every surface, rather than a rate only the engine gate could explain. Design doc 054. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four layers, per design doc 054's validation plan. Count service: distinct-PR counting, window boundary (inclusive at the edge), `applied`-only, repo scoping, empty and disabled windows — and the login-case test, which is the one keeping `lower()` in place. A regression there fails open, returning zero for a capitalized reviewer so their limit never fires, which is invisible in every surface. Engine: the strict `<` boundary the probe's own 3b/4 cross-check tripped on (reaching the limit is allowed, exceeding it is not), the two gates composing without replacing each other, `at_rate_limit` in the trace, and the guard that a single run cannot overrun the weekly cap. Integration: a reviewer at their limit receives nothing from the push despite entirely free concurrent capacity — the case `maximum_capacity` never covered — while `suggest_prs_for_reviewer` still serves them the full list and reports the spent budget honestly. Surfacing and form: the load line is unchanged for a reviewer with no limit, setting and clearing round-trips, 0 is rejected, and the help text carries the reviewer's own intake and the rolling-window wording. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Marks chunks 1-8 done and moves the doc's status from Draft/Proposed to implemented-but-inert: no reviewer has a limit set, which is also the whole rollout mechanism (no feature flag, Open Question 5). Still outstanding: a pilot cohort, and the engine simulation Open Question 3 wants before any global default. Records the three places the implementation is sharper than the plan was — the count landing in `build_reviewer_catalog` so the gate and the surfacing agree structurally rather than by discipline, `simulated_this_run` as a profile field instead of a dict threaded through five signatures, and "last 7 days" over "this week" — plus what was and was not verified. Adds `assignment_rate_limit.py` to the analyzer service list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rendering the preferences page showed the rate-limit field landing directly right of `maximum_capacity` in the 2-column grid — the correct cell, stock beside flow — but with 247 characters of help text against its neighbour's none. `maximum_capacity` has never had help text; adding an annotated field next to it made that read as an oversight, and left nothing on the page saying these are two different *kinds* of limit. So: give `maximum_capacity` a one-line explanation naming it as the concurrent hold, and cut the rate limit's text from 247 chars to 113 (168 with the measured intake sentence) by dropping the part that restated the label. The label already says "per 7 days"; the help now carries only what a number cannot — that the window is rolling rather than a calendar week, that blank means unlimited, and that self-requested PRs never count against it. Every other help text on the page is 59-97 characters, so the pair now differs by about a line instead of four. Design doc 054. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`stale_nudge_days` sat under Notifications and `auto_unassign_days` under Auto-Assignment, but they are one escalation ladder: nudge at X consecutive queue days, unassign at Y, `parse_notification_policy` clamps Y > X, and `clean()` rejects the pair otherwise. Split across sections, the resulting error — "Auto-unassign days must be greater than stale nudge days" — rendered on a field whose partner was off screen. The obvious repair, moving auto-unassign under Notifications, would have been wrong. `needs_auto_unassign` is computed with no reference to the reviewer's `notifications_enabled`; only the global enforcement flag gates it, while `needs_nudge` is a notification and does honour the toggle. Filing the unassign under "Notifications" would tell a reviewer that switching notifications off stops them being unassigned, which is false. The pair fits neither neighbour, which is why it got split, so it gets its own "Stale PRs" section between them, and each half's help text now names the switch that governs it. Auto-assign and the notifications toggle become full-width master switches, leaving no orphan half-cells in the grid. No behavior change: same fields, same validation, same storage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Status moves from implemented to deployed. The deploy needed no settings change: the one new setting defaults to 7 and there is no feature flag, so it was a schema change plus inert code. Records the one genuine hazard — an unapplied `core.0008` would take the nightly run, the console, `assigned-prs` and `suggest-prs` down together, since `build_reviewer_catalog` selects the new column — and that Heroku's release phase makes it self-guarding. Corrects the Surfacing section to the copy that actually shipped: `last 7 days` rather than `this week`, both the under-limit and spent-budget lines, and the `maximum_capacity` help text that rendering the page turned out to require. The stock/flow distinction this doc is built on has to be legible where a reviewer sets both, not only in the engine. Fixes two claims the body was still carrying that its own progress notes had already overtaken — §§3c, 6d and 4b are all run, so every figure in the Measured Baseline is measured rather than projected. The equivalent line inside the dated second-run note is left alone; it was true when written. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This PR adds a reviewer preference which allows setting the max number of PRs to be assigned in the last 7 days, which can help reviewers from being overwhelmed.