Add ReviewSLA step 1: first-response clocks and report mode - #43
Merged
Merged
Conversation
staflsystemsci
Bot
requested review from
a team,
fcheein,
gretl-silva-stafl,
mament0,
mns4595,
mzhu48,
nehalkpatel and
thebhef
and removed request for
a team
October 1, 2026 22:54
Member
Author
This was referenced Oct 1, 2026
|
Review SLA Nobody owes a review on this PR right now. Due times are in business hours, 10:00 to 17:00 Pacific: 4 h for a PR under 250 added lines, 7 h otherwise, and 4 h for a re-review. Mid-stack, an assignee owes a response on the lowest PR they haven't approved. Dashboard |
nehalkpatel
approved these changes
Oct 2, 2026
Member
Author
Merge activity
|
Rebuilds each PR's first-response clock from its ready and draft events and its reviews, two ways: the RFC's wall-clock baseline method, and the SLA's rules in business hours (10:00 to 17:00 Pacific, business days) against a 4 h or 7 h target. report.js runs it over a date range and writes weekly numbers, the way CIMetrics does for CI minutes. Nothing is scheduled yet. Business hours live in ReviewConfig/hours.js, next to the holidays and PTO calendar the actions already share. The test workflow now covers ReviewSLA. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
willson556
added a commit
that referenced
this pull request
Oct 2, 2026
…rd (#44) Step 2 of Proposal 2's rollout ([rollout plan](https://claude.ai/code/artifact/12932aa2-661e-44c3-a59e-d17a639f6b95)): the ReviewSLA action. Merging it changes nothing by itself. The scheduled workflow that runs it lives in a private repo, not this one, because this repo's run logs are public and the job's would show who is out of office and activity in the private repos. The README has the workflow. It's skipped until that repo's `REVIEW_SLA_MODE` variable is set, and setting it back to `off` is the rollback. Every 30 minutes during business hours (10:00 to 17:00 Pacific, business days), it reads the open PRs in the five pilot repos and this one. From their timelines it rebuilds each assignee's clock (`ReviewSLA/owed.js`): - **First response:** 4 business hours under 250 added lines, 7 otherwise. It starts at the latest of: the PR marked ready, the person assigned, and, mid-stack, their approval of the PR below or that PR merging. - **Stacks:** an assignee owes only on the lowest ready PR they're assigned to and haven't approved. Requesting changes doesn't move them up. - **Re-review:** 4 business hours from a re-request, but only if the author has pushed since that reviewer's last review. Graphite's re-requests on `gt submit` start nothing. - **Drafts** stop every clock on the PR, and marking it ready again starts them over. What each mode does: | Mode | Effect | | --- | --- | | `shadow` | A Review SLA comment on each PR where someone owes a review (who owes what, due when), the Confluence dashboard and the 10:00 Slack digest | | `remind` | Also a Slack DM when a review is due, and reassignment of anyone the PTO calendar has out before their review is due | | `enforce` | Reassignment at twice the target is step 3; for now it warns and runs as `remind` | Details: - **Reminders are sent once.** The comment's hidden marker records each reminder. The comment is written before the DM goes out, so a failure means a missed reminder, not a repeated one. Runs don't overlap. - **Out-of-office reassignment** replaces only the person who's out, on every PR in the stack they're assigned to. A domain approver is replaced from the domain team, a rotation reviewer from the rotation team. A comment on the lowest of those PRs says who took over. The least-loaded pick moves from `AssignReviewers` to `ReviewConfig/pick.js`, so both pick the same way. - **The digest** comes from the 10:00 Pacific cron, one for daylight time and one for standard time, so a late scheduled run still posts it once. - **The dashboard** stores a hash of its content in the page version's message. It saves only when that changes, as a minor edit, because Confluence rewrites the page body on save. - **API errors are warnings,** and that PR, repo, channel or page is skipped for the run. - **Re-review** compares the PR's head commit with the commit each review was left on, not commit dates. Commit dates need the app's Contents permission in private repos, and a rebase rewrites them. Setup: the org secrets and variables in the README are all set, including `CONFLUENCE_URL` and `CONFLUENCE_USER`. Still to do: the private repo with the workflow and its `REVIEW_SLA_MODE` variable. Testing: - `node --test AssignReviewers/ ReviewConfig/ ReviewSLA/`: 81 cases, 26 of them new. - The clock rules: the 16:30 Friday small PR, the three-PR stack, approving the bottom, requesting changes, a draft below, the PR below merging, re-review with and without a push, draft and ready again, late assignment, and bots and merge queue. - Whole runs against a fake org: `off`, outside business hours, shadow vs remind, one DM per clock across runs, reassignment across a two-PR stack, someone out only after their due time, the digest cron in daylight and standard time, the dashboard saving only on change, an unreadable repo, a failed comment write sending no DM, a missing Slack ID, and `enforce`. - Dry run of the GitHub query and clocks, read-only, against all six repos today: 197 open PRs read. 2 clocks are running, both on this repo's own stack. StaflLib and coit-tower have 6 ready PRs with assignees, all already reviewed. - A `shadow` run in Actions from a throwaway branch (since deleted) read the open PRs, wrote the Review SLA comments on #41 and #43, and saved the Confluence dashboard. Before the commit-ID change, the private repos failed with "Resource not accessible by integration". Slack hasn't been called for real yet. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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
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.

Step 1 of Proposal 2's rollout (first-response SLA, rollout plan): the clock code and report mode. Nothing is scheduled, so nothing changes for authors or reviewers. The scheduled job, with reminders and the Confluence dashboard, comes in step 2's PR.
ReviewSLA/clock.jsrebuilds a PR's first-response clock from its ready and draft events and its reviews. It keeps two measures:ReviewSLA/report.jsis report mode. Like CIMetrics, it runs locally withgh, and it writes weekly numbers per repo plus a per-PR TSV for spot checks. Events after--untilare ignored, so a past range reads the same whenever it's run.ReviewConfig/hours.jsadds business hours (10:00 to 17:00 Pacific, minus weekends andholidays.json), next to the PTO calendar code it builds on.pto.jsnow exports its date helpers.ReviewSLA/.Step 1's check: over Jun 26 to Sep 24, does the RFC measure reproduce the RFC?
For the PRs the RFC measured, every timing matches its source data to within 0.1 h. The full run times 34 more StaflLib PRs. The RFC sampled StaflLib's 300 most recent PRs by creation date, and these were opened before Jun 26 but marked ready during the range.
The business-hours baseline for the gates, over the same range:
Testing:
node --test AssignReviewers/ ReviewConfig/ ReviewSLA/: 55 cases, 21 of them new. They cover the plan's clock cases that don't need stacks or assignees: 16:30 Friday to 13:30 Monday, a holiday, which reviews count, draft and ready again, a PR reviewed as a draft, unreviewed PRs before and after their target, a PR closed unreviewed, events after the range, the switch to standard time, and bot and merge-queue PRs.🤖 Generated with Claude Code