feat: ask reviewers to confirm assignments, reviewer console page#188
Open
bryangingechen wants to merge 33 commits into
Open
feat: ask reviewers to confirm assignments, reviewer console page#188bryangingechen wants to merge 33 commits into
bryangingechen wants to merge 33 commits into
Conversation
Add a living implementation plan for a pre-assignment acceptance gate:
proposals are offered to reviewers who must accept via a GitHub-login
console before the assignment is executed on GitHub.
Key decisions captured:
- Per-reviewer assignment_acceptance mode (auto/confirm), orthogonal to
auto_assign and notifications; new accounts default confirm, existing
backfilled auto.
- Confirmation prompts are transactional and decoupled from
notifications_enabled (which governs only optional nudges).
- {unassigned, proposed, assigned} state model with legibility invariants;
on-queue-exit policy resolved to invalidate but pluggable via a single
proposal_validity predicate + setting.
- Proposal state surfaced on the board and in pr-info; history retained
indefinitely (low volume, analytical value).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014dQnn9K9BnaFA12a2jVNKy
Chunk 1 of design doc 050 (reviewer assignment acceptance gate). - ReviewerPreference.assignment_acceptance CharField (choices auto/confirm, default confirm) + ACCEPTANCE_* constants. - Migration 0007: AddField (future rows default confirm) + RunPython backfill flipping all existing rows to auto, grandfathering current reviewers. - Admin: field in list_display/list_filter + bulk actions to set the mode (the pool-flip lever). - Tests: field default, backfill function, and importer no-clobber invariant (the importer never touches the field, so a re-import cannot flip an existing reviewer's mode). Update the living doc: mark Chunk 1 landed with a progress note. Validation: 5 new tests + full core suite (55) green on Postgres; makemigrations --check clean; ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014dQnn9K9BnaFA12a2jVNKy
Chunk 2 of design doc 050. - analyzer.AssignmentProposal (migration 0031): repository/pr_number/ reviewer_login/snapshot, state (proposed/accepted/declined/expired/ superseded), expires_at/decided_at/notified_at/decided_via. created_at is the proposal time (no redundant proposed_at). - Partial unique constraint enforcing one active (proposed) proposal per PR — the "one at a time" invariant, race-safe at the DB level. - ReadOnlyAdmin registration. - Backup policy: analyzer_assignmentproposal added to BACKUP + TRUNCATE (retained in real backups, truncated from the sanitized public dump like the sibling reviewer tables). - Tests: defaults, one-active-per-PR enforcement, terminal states don't block re-proposal, distinct-PR proposals allowed. Update the living doc: mark Chunk 2 landed. Validation: 4 model tests + full analyzer suite (403) green on Postgres; backup-policy validator passes; makemigrations --check clean; ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014dQnn9K9BnaFA12a2jVNKy
Chunk 3 of design doc 050. Insert the three proposal-aware additions into the reviewer assignment builder, all routed through a new shared `_prepare_assignment_inputs` helper so the builder and the diagnostic `build_reviewer_assignment_trace` cannot diverge: - Candidate exclusion: withhold PRs that already have an active (`proposed`) proposal from the candidate pool (one proposal per PR at a time). - Pending-load contribution: a reviewer's active proposals add weighted load via the new pure engine seam `add_pending_proposal_load` (weight ANALYZER_ASSIGNMENT_PROPOSAL_PENDING_LOAD_WEIGHT, default 1.0; a proposal occupies a capacity slot, open-list/total untouched). - Cooldown exclusion: reviewers with a recently `expired` proposal for a PR are excluded for it within ANALYZER_ASSIGNMENT_PROPOSAL_EXPIRE_COOLDOWN_DAYS (default 14; 0 disables), merged into the per-PR exclusion set alongside opt-outs. The integration is data-driven and ungated (no proposals => no-op), matching the opt-out precedent; the master kill switch and propose task land in Chunk 4. AreaStatsBuilder is intentionally left unchanged. Adds the two tuning settings to settings/base.py and .env.example. Validation: 9 new tests (2 pure-engine, 7 builder/trace) + full analyzer suite (412) green on Postgres; makemigrations --check clean; ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Chunk 4 of design doc 050. Split the 046 apply path into a batch (propose) half
and a reusable mutation half, and build the acceptance-gate propose pipeline.
- Extract assign_reviewer_and_record + latest_default_snapshot +
parse_snapshot_assignments into reviewer_assignment_apply and refactor
apply_assignments_for_repo onto them (behavior-preserving; 21 apply tests green),
so the auto/fallback direct-assign, the legacy apply sweep, and the future console
accept share one GitHub mutation + ReviewerAssignmentApplication audit trail.
- propose_assignments_for_repo + analyzer.propose_reviewer_assignments task +
propose_reviewer_assignments management command. Per snapshot {pr: login}, branch on
ReviewerPreference.assignment_acceptance: auto (and confirm reviewers with no Zulip
link) direct-assign; confirm + Zulip-linked create an AssignmentProposal. Re-validates
live state, dedupes on active proposal / recently-applied, clamps the per-reviewer
acceptance window to >=7 days, applies a GitHub-mutation-only per-repo cap (defers
assigns without starving DB-only proposals), and has a fully side-effect-free dry-run.
- Centralize on-queue-exit/expiry in the single assignment_proposal_validity.proposal_validity
authority (assignee-landed & closed/merged -> superseded; timeout -> expired; open +
off-queue -> superseded under invalidate / live under retain; on_queue=None never
invalidates) and drive analyzer.expire_assignment_proposals from it: ungated essential
maintenance, no GitHub writes, off-queue invalidation only from a fresh snapshot,
idempotent conditional UPDATE ... WHERE state='proposed'.
- Add rollout flags (ENABLED master switch, DELIVERY_ENABLED, ASSIGN_ON_ACCEPT_ENABLED,
DRY_RUN), WINDOW_DAYS, ON_QUEUE_EXIT, and propose/expiry schedules to settings/base.py,
.env.example, and beat. propose supersedes apply -- enable one, not both.
- Update analyzer/AGENTS.md task surface.
Validation: 42 new tests + full analyzer suite (454) green on Postgres;
makemigrations --check clean (no model changes); ruff clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Chunk 6 of design doc 050 (built before Chunk 5 so the notification DM links to a live surface). New console/ Django app at /console/ where a confirm-mode reviewer signs in with GitHub and accepts/declines the assignment proposals made to them. - Centralized site base URL: new canonical QUEUEBOARD_BASE_URL; legacy ZULIP_PREFS_URL_BASE falls back to it; core.services.site_urls is the single resolver (prefs/registration links inherit the fallback for free). - Shared OAuth-state primitive in core (core.services.oauth_state): Fernet encrypt+integrity+TTL over a JSON dict, plus a token-less console helper carrying a CSRF nonce + next. Refactored zulip_bot.registration_oauth_state to delegate to it (public API unchanged). Zulip-agnostic core.services.github_identity maps the OAuth identity to a core.User without touching Zulip fields. - console app: login/oauth_callback/logout (stable bookmarkable URL, ?next=, nonce CSRF), a list view (pending proposals + matched labels + expiry + per-repo load), and POST accept/decline. Both re-validate via the single proposal_validity authority and render "no longer available" (retiring the stale row) when a proposal is no longer actionable; a reviewer can only act on their own proposals. Accept (gated by ANALYZER_ASSIGNMENT_PROPOSALS_ASSIGN_ON_ACCEPT_ENABLED) reuses the verbatim 046 mutation (assign_reviewer_and_record) then marks accepted; when off it leaves the proposal pending. Decline marks declined + upserts a ReviewerOptOut. - Fix a pre-existing red test from Chunk 1: zulip_bot prefs-form field coverage now accounts for assignment_acceptance (admin-managed, not self-serve). - Correct stale OAuth path in design doc 050; add console/AGENTS.md + CLAUDE.md; list the app in qb_site/AGENTS.md. Validation: 25 new tests + console/core/zulip_bot suites (387) green on Postgres; makemigrations --check clean (no models); ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to the acceptance-gate console (design doc 050), preparing for deploy
and for continuing Chunk 5 in a fresh session.
- Fix a phantom setting: CONSOLE_OAUTH_STATE_TTL_SECONDS was read via
getattr(settings, ...) but never wired to os.getenv/base.py or .env.example
(so it was silently uneditable). Wire it through base.py + .env.example.
- Surface the live-deployment adjustments the console needs, where operators look:
- QUEUEBOARD_BASE_URL (new canonical site base) documented in .env.example and
docs/zulip_github_oauth_setup.md, plus a deploy-prereqs note in design doc 050.
- The GitHub OAuth App callback must permit /console/oauth/callback/; register the
callback at the site root so both the registration and console paths are covered.
Documented in the OAuth runbook (was registration-only) and design doc 050.
- Strengthen the "settings must be in base.py AND .env.example" rule in the root
AGENTS.md (add the getattr-phantom antipattern) and add a Settings & .env Hygiene
section to qb_site/AGENTS.md, since this step is often forgotten.
- Record the resolved Chunk 5 (notification) build plan in design doc 050 so it can
be picked up in a fresh session: separate delivery task gated by
ANALYZER_ASSIGNMENT_PROPOSALS_DELIVERY_ENABLED, notified_at as the dedup key,
console link via build_site_url, and the narrow attention-ping suppression rule.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per-reviewer Zulip DM digest of pending, not-yet-notified assignment proposals, linking to the console. Dedupe carried by AssignmentProposal.notified_at (stamped race-safely after a successful send); one DM per reviewer across all repos. Requires both ANALYZER_ASSIGNMENT_PROPOSALS_ENABLED and _DELIVERY_ENABLED to send; _DRY_RUN previews. Also suppress the reviewer-attention "newly assigned" ping when the assignee arrived via a recently accepted-via-console proposal, so the proposal DM is the single touch for that lifecycle. - analyzer.deliver_assignment_proposals task + service + command - ANALYZER_ASSIGNMENT_DELIVER_* schedule settings + beat + .env.example - design doc 050 Chunk 5 marked landed; analyzer AGENTS.md task surface Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Surface the acceptance-gate {unassigned, proposed, assigned} state on the
board/API, in single-PR queue info, and in the Zulip pr-info command —
always distinct from GitHub assignees (a proposal is not an assignee).
- queueboard snapshot: each PR entry gains a `proposal` field
({reviewer, expires_at} or null) from one batched state=proposed query;
the raw payload is served verbatim by the API.
- PRQueueInfo: proposed_to + proposal_expires_at, read live via a single
indexed point query in both the snapshot and DB paths.
- pr-info command: distinct "Proposed to X (awaiting acceptance, expires …)"
line; proposed login resolved through the silent-mention map.
AssignmentProposal ReadOnlyAdmin (Chunk 2) already provides the retained
history changelist. design doc 050 Chunk 7 marked landed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
All build chunks (1-8) for the reviewer-assignment acceptance gate are landed; the feature is pending staged rollout (flags default off) and a staging end-to-end validation. - root AGENTS.md: list qb_site/console/ in the AGENTS-locations pointer - analyzer AGENTS.md: snapshot `proposal` field + pr_info proposed_to notes - zulip_bot AGENTS.md: pr-info proposal-line note - design doc 050: mark Chunks 7-8 landed, converge status, add an operator rollout checklist Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The console accept handler treated `assign_reviewer_and_record`'s
`already_recorded` outcome as success and marked the proposal `accepted`
("you are now assigned"). But `already_recorded` only means a row for
(run_date, repo, pr, reviewer) already exists — which is a success *only*
when that row is APPLIED. A prior same-day FAILED attempt (e.g. an earlier
click GitHub rejected) also returns `already_recorded`, so a retry — which
the 502 message explicitly invites — would falsely tell the reviewer they
were assigned with no GitHub assignee.
`assign_reviewer_and_record` now returns the existing record on
`already_recorded` (was `None`) so the caller can inspect its status; the
console marks accepted only when the assignment actually landed (`applied`,
or `already_recorded` with an APPLIED row) and otherwise leaves the proposal
pending with a "try again" page. The batch apply/propose callers ignore the
third return value, so their behavior is unchanged.
Adds console tests for the failed, already-recorded-FAILED, and
already-recorded-APPLIED accept paths.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a reviewer accepted a proposal for a PR they already had an active ReviewerOptOut on (e.g. a GitHub self-unassign reconciled into an opt-out), the handler rendered "you have opted out" but retired the proposal with a no-op verdict (terminal_state=None), leaving it `proposed`. Because proposal_validity does not consult opt-outs, the expiry sweep wouldn't clear it either, so it lingered as "proposed" on the console and board until it timed out. Retire it to `superseded` (decided_via=sync_superseded) instead, and reword the message. Adds a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`/console/` is a public URL and its OAuth callback called resolve_or_create_user_from_identity, minting a core.User for any GitHub account that authorized the app — an unbounded row-creation surface for unrelated strangers. Add a `create` flag to resolve_or_create_user_from_identity (default True, returns User | None) and have the console call it with create=False: an identity we have never seen (not registered via the Zulip flow, not ingested by the syncer) now gets a 403 "registered reviewers only" page and no row, instead of a session. Known reviewers are unaffected. Adds console (unknown-login denied) and core-helper (resolve-only) tests; documents the create=False gating in console/AGENTS.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Console home improvements (design doc 050 review): - Group pending proposals under a per-repo heading, each with its own load line (assigned + pending / capacity), instead of a flat list plus an unlabelled load summary — a reviewer active in multiple repos can now tell which repo each line belongs to. Groups are ordered by (owner, name). - Fix an N+1: batch the proposed PRs (one query per repo) and their labels (one query) rather than a PullRequest + PRLabel query per proposal. - Render the expiry in the viewer's own timezone: the server emits a UTC ISO-8601 timestamp in a <time datetime> element and a small inline script reformats it client-side via Intl.DateTimeFormat, keeping a "… UTC" text fallback when JS is off (was a hardcoded "UTC" label only correct while TIME_ZONE=UTC). Updates the home-listing test and adds a multi-repo grouping test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two security fixes to the console GitHub-OAuth path (review findings): - resolve_or_create_user_from_identity no longer resolves a login-only match whose stored github_node_id is non-empty and differs from the authenticated identity's node id. Previously a recycled GitHub username (old reviewer renamed away, stranger registered the freed login) resolved to the previous owner's core.User, handing the new account holder the reviewer's console session and proposals. The registration linker already rejected this case; the console resolver now does too. - console session set_reviewer now rotates the session key (cycle_key) on login promotion, like django.contrib.auth.login, so a pre-authentication session key planted in the browser cannot be replayed after the victim signs in (session fixation). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Proposals store GitHub's canonical (mixed-case) login, but every other ReviewerOptOut writer/clearer (syncer unassign upsert and assign-time clearing, opt-out backfill) lowercases against the table's case-sensitive unique constraint. A console decline for a mixed-case login therefore created a second, permanently-active row the syncer's exact-match clearing could never deactivate, leaving the reviewer excluded from the PR even after a later assign/unassign cycle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both analyzer.apply_reviewer_assignments and analyzer.propose_reviewer_assignments default to the 00:45 UTC beat slot, and the 'enable one or the other, not both' rule existed only in comments. A deployment that turned on the acceptance gate while the legacy apply flag was still set would have the proposal-unaware apply task direct-assign confirm-mode reviewers past the gate, alongside a dangling AssignmentProposal for the same (pr, reviewer). Now the apply task skips itself (reason superseded_by_proposals_pipeline, error log) when ANALYZER_ASSIGNMENT_PROPOSALS_ENABLED is set, and the propose task logs a warning about the misconfiguration. Docs updated to say the rule is enforced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Propose-time re-validation only skipped a PR when its assignees intersected eligible reviewer logins, while proposal_validity supersedes a pending proposal on ANY non-empty assignee set. A PR whose sole assignee is a non-reviewer (bot, unregistered self-assign) would get a proposal created at 00:45, DM'd at 01:00, retired as superseded by the hourly sweep, then re-created by the next daily run — superseded rows feed neither the re-propose cooldown nor the active-proposal exclusion — DMing a reviewer about the same unacceptable PR every day. Any assignee now blocks creation, so a proposal is never minted that the validity authority would immediately retire. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The console duplicated the expiry sweep's fact-gathering verbatim: _queue_membership (rule-set cache key, snapshot lookup, freshness test, Queue/known-PR extraction), the SNAPSHOT_FRESH_SECONDS constant, and the pr_state/current_assignees/on_queue derivation — including the safety-critical 'stale snapshot must never mass-invalidate' rule (on_queue=None fallback). Two copies of the inputs defeat the point of the single proposal_validity authority: the verdict was shared but the facts it was fed could drift. Move queue_membership and a live_proposal_validity helper (facts assembly + predicate call) into assignment_proposal_validity, and consume them from both the expiry sweep and the console. No behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Opt-out invalidation of a pending proposal existed only inline in the console accept handler; the proposal_validity predicate and the expiry sweep were opt-out-blind. A reviewer who acquired an active ReviewerOptOut after their proposal was created (a GitHub self-unassign reconciled into an opt-out, or the backfill command) kept the doomed proposal 'proposed' on the console, pr-info, and the board for up to its full multi-day window — it was retired early only if they happened to click Accept. proposal_validity now takes an opted_out fact (new reason opted_out, retiring to superseded), live_proposal_validity derives it from the shared _opt_outs_for_prs map, the expiry sweep passes active opt-outs, and the console's hand-built synthetic ProposalValidity block is deleted in favor of the shared path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AreaStatsBuilder fed compute_area_stats the raw collect_assignment_statistics output, without the add_pending_proposal_load step that _prepare_assignment_inputs applies for the assignment builder and trace. A reviewer whose capacity was fully occupied by pending proposals therefore read as available (at_max_capacity=False) on the area-stats surface while the very next assignment run would assign nothing in those areas. Design doc 050 states a pending proposal counts toward load with no carve-out for area stats. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
_resolve_window_days applied the MIN_WINDOW_DAYS=7 floor to the operator-configured ANALYZER_ASSIGNMENT_PROPOSAL_WINDOW_DAYS as well, although base.py, .env.example, and design doc 050 all document the >=7 clamp as applying only to the per-reviewer notification_settings override. An operator piloting the gate with a short window (e.g. 3 days) silently got 7-day expiries, occupying reviewer pending load longer than configured with no warning. The global value is now honored as-is (floored at 1 day); per-reviewer overrides keep the weekly floor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The console-accept de-dupe suppressed the 'newly assigned' ping for any (pr, reviewer) pair with an accepted proposal in the ping window, without checking that the assignment event being pinged was the one the acceptance produced. A reviewer who accepted, self-unassigned, and was manually re-assigned within 24h silently lost the ping for the unrelated re-assignment; conversely an acceptance just before the window whose ASSIGNED event landed just inside it produced a duplicate ping. Suppression now matches the latest console-accept decided_at against the assignment event's occurred_at within a 1h tolerance (the accept performs the GitHub assign moments before decided_at is stamped), and the acceptance query is widened by the same tolerance so boundary acceptances still suppress their own assignment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
_split_message_chunks existed as three near-identical copies (assigned-prs command, reviewer attention DMs, assignment-proposal digest), and the newest copy — the proposal digest — was the weakest: it lacked the hard-split of a single line longer than max_chars, so one oversized line would produce a chunk Zulip rejects and fail the whole digest send. Consolidate into split_message_chunks (+ shared MAX_MESSAGE_CHARS) in zulip_bot.services.zulip_client, next to the client every caller uses. The shared version also hard-splits an oversized line regardless of buffer state (the old copies only hard-split when the line arrived with an empty buffer), so no chunk can ever exceed the ceiling. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
queue_membership loaded the entire QueueSnapshot payload — complete entries for every open PR, multi-MB on repos like mathlib4 — per repo on every hourly expiry sweep run (whenever pending proposals exist) and on every console accept/decline POST, just to read the Queue number list and the known-PR keys. Extract exactly those two fragments in SQL (jsonb path operator + jsonb_object_keys), so only a few KB transfer. Postgres-only operators, matching the repo's Postgres-only runtime policy. Behavior is unchanged: an empty/missing payload still yields empty sets, and empty known_prs already forced on_queue=None downstream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
resolve_or_create_user_from_identity defaulted to create=True, but its only production caller (the console) passed create=False — the default was both dead and the dangerous path: a future caller forgetting the flag would silently mint core.User rows for arbitrary GitHub accounts, exactly what the console gating exists to prevent. Replace it with resolve_user_from_identity (no flag, no creation branch). The race-safe savepoint create pattern remains available in registration_linking and core_entities_sync for callers that legitimately create users; a create-on-miss variant can be added as an explicit separate entry point if one is ever needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rewrite the living implementation plan into ADR shape per docs/design-decisions/README.md: Context / Decision / Consequences / Operational Notes / Deferred Follow-Ups / Alternatives. Collapses the chunk plan and progress log, and updates every statement to the as-built architecture, including the review-hardening pass (enforced apply/propose mutual exclusion, opt-out-aware shared validity with shared input assembly, any-assignee propose rule, resolve-only console identity with recycled-login guard and session-key rotation, normalized opt-out writes, per-reviewer-only window clamp, pending load in area stats, shared Zulip chunker, SQL-side queue-membership extraction). Rollout checklist and staging end-to-end check retained as pending operational steps. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replies in place with the stable, token-less reviewer console URL
(build_site_url(reverse("console:home")), design doc 050) where a
reviewer accepts or declines assignment proposals. The link is
non-secret and identical for every reviewer since the console
self-authenticates via GitHub OAuth, so it is an in-place reply rather
than a proactive DM. Like all commands it is reachable only where
ZULIP_COMMAND_POLICY permits it.
Also records the stable /prefs URL (shared console session) idea as a
tracked follow-up in doc 050.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Complete the QUEUEBOARD_BASE_URL consolidation that doc 050 started, and remove two dead settings. With a single deployment there is no need for the legacy per-feature base URLs or their fallbacks. - GITHUB_OAUTH_REDIRECT_URI: the registration callback is now derived from QUEUEBOARD_BASE_URL (via resolve_site_base_url), mirroring the console. The local-dev request-host fallback is kept for when no base URL is configured. - ZULIP_PREFS_URL_BASE: the four link builders (prefs/registration/close-pr/ label-pr) now resolve through build_site_url instead of reading the setting directly, so QUEUEBOARD_BASE_URL is the sole base. Dropped the legacy fallback branch in site_urls.resolve_site_base_url. - ZULIP_ASSIGNMENT_SUCCESS_EMOJI: genuinely dead — its reader was removed in 3bd8cc8 when assign/unassign switched to returning assignees text. - base.py: replace the tautological `if 900 > 0` guard around the collect_syncer_metrics beat entry with an unconditional registration. Updated the OAuth setup runbook, console/AGENTS.md, and doc 050 to match; both OAuth callbacks now build from QUEUEBOARD_BASE_URL. Tests that overrode the removed settings switched to QUEUEBOARD_BASE_URL (same derived values); the two legacy-fallback tests were removed. Verified: ruff check/format clean; zulip_bot + console + core suites pass (402 tests). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Expose ReviewerPreference.assignment_acceptance (auto/confirm) as a two-option radio in the Zulip prefs form's Auto-Assignment section, so reviewers can opt into the propose-then-accept flow without an admin. The control is only rendered when ANALYZER_ASSIGNMENT_PROPOSALS_ENABLED is on: the form pops the field from self.fields when the flag is off, so it is never rendered, validated, or saved, the stored value is preserved, and a crafted POST cannot flip the mode. This replaces explaining the gating caveat in help text. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Build on the reviewer_load service (PR #189) for the acceptance-gate console. Pending-proposal load (design doc 050): - build_reviewer_loads now folds a reviewer's active proposals into current_load (weight from ANALYZER_ASSIGNMENT_PROPOSAL_PENDING_LOAD_WEIGHT), exactly as the assignment engine / area stats already count them. assigned_open still excludes proposals (a proposal is load, not an assignee). Data-driven: no proposals => no change. Adds a public pending_proposal_load_for_repo() wrapper so reviewer_load does not import private helpers. - Consequence: the assigned-prs command and daily digest now count pending proposals in their load too, so the figure matches the engine on every surface. Reviewer console home is now a per-repo dashboard: - Load line is computed via the shared helper OUTSIDE the proposal loop, so it renders even with zero proposals. Removes the ad-hoc, case-sensitive assignees__contains count and the hand-rolled assigned+pending/capacity math. - Each active repo (has a proposal OR >=1 assigned open PR) shows: load line + assigned open PRs with status (from build_reviewer_attention_reports) + the proposals to accept/decline. New empty state covers "nothing here". Testing: uv run ruff check/format; analyzer + zulip_bot + console + core suites (922 tests) green against dockerized Postgres. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bryangingechen
force-pushed
the
feat/reviewer-assignment-acceptance-gate
branch
from
July 13, 2026 03:49
d04e0f5 to
9c857d0
Compare
Uniform look with the other reviewer-facing pages, plus three self-service actions on the console. Styling - Promote shared_pages.css to app-neutral qb_site/static/shared/ as one source of truth and repoint the 7 zulip_bot templates. Console templates rebuilt on that design system (hero/card/cta/status/pr-label, Work Sans, teal palette) via base.html + a new console/console.css for console-only bits; dropped the bespoke inline styles and dark mode (the shared system is light-only, matching the siblings). Actions - Assign-anyway: an escape hatch on the "no longer available" page that deliberately bypasses the validity gate (precondition: PR open and the reviewer isn't already an assignee — which is exactly the recoverable set), reusing the ASSIGN_ON_ACCEPT flag and the shared assign_reviewer_and_record path, and clearing any active per-PR opt-out on success. - Self-unassign: the assigned-PR roster becomes a checkbox form that removes only the signed-in reviewer (never a login from the request), gated by ANALYZER_ASSIGNMENT_PROPOSALS_CONSOLE_UNASSIGN_ENABLED (off by default). - Per-PR load breakdown (+1 / +0.1 / +0) from reviewer_load.pr_load_breakdown, folded over the same snapshot as the aggregate so the parts sum to the load line; explained by a legend revealed by clicking the load line (collapsed by default). - The "no longer available" and "unassigned" screens link out to the GitHub PR, woven into the message text. Settings wired through base.py + .env.example. Docs updated (console/AGENTS.md, design doc 050). Tests: console + analyzer reviewer_load suites pass (60); ruff check + format clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bryangingechen
changed the title
Pending acceptance-gate proposals (design doc 050) now surface as a "Proposed to you, awaiting your response" section of the daily reviewer-attention DM instead of a separate digest task. An un-notified proposal triggers a send even for reviewers with notifications muted (proposal prompts stay transactional); the muted DM carries only the proposal section. Dedupe stays on AssignmentProposal.notified_at, stamped after a successful send, so a proposal is announced once and then rides along as context when other triggers fire. Removes analyzer.deliver_assignment_proposals (task, service, management command, beat entry, tests) and the ANALYZER_ASSIGNMENT_PROPOSALS_DELIVERY_ENABLED / ANALYZER_ASSIGNMENT_DELIVER_* settings. Proposal delivery now requires ANALYZER_REVIEWER_ATTENTION_ENABLED + _DELIVERY_ENABLED; docs and .env.example updated accordingly. Tested: ruff check/format clean; analyzer + console + zulip_bot suites (859 tests) pass against dockerized Postgres. Co-Authored-By: Claude Fable 5 <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.
Currently reviewers are auto-assigned once a day. This PR is centered on adding a confirmation step to assignments: PRs now go into a "propose" phase where it is proposed as a potential assignment to a single reviewer who must accept within 7 days.
As requested on Zulip (reviewers channel).