feat(scheduler): add adaptive scheduled hot-review allocation - #1110
feat(scheduler): add adaptive scheduled hot-review allocation#1110brokemac79 wants to merge 29 commits into
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
@clawsweeper review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Codex review: needs maintainer review before merge. Reviewed August 12, 2026, 7:32 PM ET / 23:32 UTC. ClawSweeper reviewWhat this changesThis PR adds adaptive capacity allocation for scheduled hot reviews, with durable observations, cursor reservations, staged rollout controls, workflow integration, tests, documentation, and a local Worker proof harness. Merge readinessKeep open for explicit maintainer product direction and a current-main rebase. The branch is a substantial default-disabled scheduler feature, not a fix for an established contract; GitHub reports it as dirty, so its exact-head proof does not establish the behavior of a merge result. Priority: P2 Review scores
Verification
How this fits togetherScheduled target fanout selects repositories and sends review work into the durable exact-review queue. This PR inserts an adaptive planning and telemetry layer before that dispatch path while retaining legacy selection by default. flowchart LR
A[Scheduled target fanout] --> B[Queue and token facts]
A --> C[Repository observations]
B --> D[Adaptive allocation]
C --> D
D --> E[Cursor reservation]
E --> F[Repository review dispatch]
D --> G[Durable status snapshot]
Decision needed
Why: This is a new cross-lane scheduling policy with durable state, rollout configuration, and operational consequences; current source does not define it as a broken existing contract, and VISION.md reserves product direction for human judgment. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: If maintainers sponsor adaptive scheduling, rebase it onto current main, reduce any unnecessary control-plane scope, and rerun the exact-head runtime proof and review on the resolved branch; otherwise retain the existing legacy scheduler and quota controls. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR proposes a new scheduling capability rather than reporting a current-main bug with a failing reproduction. Is this the best way to solve the issue? Unclear: the design has strong exact-head proof, but the large deterministic cross-lane control plane needs maintainer sponsorship and a fresh proof after merging current-main changes. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 9a257905e50b. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (8 earlier review cycles)
|
8901a08 to
854a29c
Compare
854a29c to
ceb3665
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Closing by maintainer direction. No current issue signal indicates that the default-disabled adaptive scheduler package remains needed, and this draft is now stale and dirty against main. The branch and PR history are retained for selective recovery if the requirement returns. |
Important
DO NOT MERGE without Martin's explicit approval. This PR is intentionally draft and will receive an independent maintainer review before any merge decision. It does not deploy or activate adaptive production behavior.
Summary
Delivers the complete adaptive scheduled hot-review package in one PR:
The merged behavior remains unchanged by default:
legacymode plus the kill switch are configured, hot fanout remains on the 20-minute containment cadence, scheduled admission remains 300/hour with burst 30, and normal fanout is unchanged.Problem
The contained scheduler gives every selected repository the same generic candidate allowance despite large differences in eligible demand, novelty, review cost, observation health, and time since service. That wastes scarce review capacity and can starve quieter repositories. A safe replacement also needs durable evidence, replayability, quota/circuit integration, explicit rollout controls, and exact rollback semantics; an allocator alone is not an operable production design.
Implementation
Allocation and pressure signals
ceil(1.5 * service capacity), capped at 30; at most 20 repositories are selected.Observation, durability, and replay
scripts/evaluate-hot-allocation.mjsreplays sanitized fixtures deterministically without GitHub, queue, or production mutation.Execution safety and cursor settlement
repository_dispatchhas no rollback or idempotency key. It remains due in inventory and is reconsidered by demand, novelty, probes, or the 24-hour fairness floor.Rollout and rollback
legacy,shadow,canary, andfull; deterministic 10/50/100 cohorts and explicit canary repositories are supported.legacyplus kill switch on. Turning on the kill switch immediately makes legacy selection authoritative without changing cadence or admission limits; rollback does not require clearing durable observations or cursors.Validation
Reviewed range:
c145b4599285969b43e40d0816071893d5090501ceb36659a534ad77f88f0573b68e4d7f3f602563Passed on the current head:
The checked-in proof suite reports 67/67 focused tests passing: 58 allocator/runtime/control-plane/fanout tests, six Worker/durable-boundary tests, and three workflow/cadence tests.
After rebasing onto #1115 and #1116, the overlap checks passed: canonical GitHub API tests (3/3), adaptive control-plane tests (3/3), selected Worker/Durable Object tests (3/3), static checks, build, lint, and the complete exact-head behavior proof. The sole rebase conflict was import-only in
dashboard/exact-review-queue.ts; both the canonical GitHub API imports and adaptive store imports were retained.Broad-check classification:
pnpm run checkpassed static checks, formatting, build, and lint, then reached repository test surfaces that require Bash/WSL, POSIX locking/symlinks, and Unix temporary-file semantics. The observed workflow failures were/bin/bash-unavailable errors from the Docker Desktop WSL distribution; those unrelated platform failures are not treated as a request to broaden this PR.concurrency.queueschema gaps that reproduce onorigin/main; the changed completion payload is covered by the focused workflow parser test.review-blob-hydrationfailures. The identical isolated three failures reproduced on then-current main51ac499c741b...in the same image; the changed scheduler/Worker paths were not involved. The final exact-head focused container proof passes, and hosted CI on the rebased head remains authoritative for the repository's supported environment.Real Behavior Proof
Claim: at the exact committed head, the complete default-disabled package authenticates and persists telemetry, survives Worker restarts, fences and atomically settles active-mode cursor batches while preserving independent shadow legacy progress, rejects reservation replay, runs the actual fanout command through zero-success abort and successful recovery, deterministically replays allocation fixtures, and exposes only bounded sanitized status.
Exercised surface: a clean checkout of the exact head inside Docker-backed Crabbox; pinned Wrangler
4.107.0; SQLite Durable Object; real signed/unsigned HTTP routes; Worker restart and persistent state; cursor reserve/commit/abort; the builtdist/repair/target-fanout.jscommand crossing loopback TLS into Wrangler; offline replay; 67 focused tests, including a signed current-policy hot-lane allocator view beyond the 100-partition public cap, nested unknown-field decision sanitization, malformed inactive rollback controls, empty-plan dependency isolation, and repeated shadow comparison-commit failure. Production activation was disconnected.Scenario/fixture: sanitized
example/alphaandexample/betarepository observations, synthetic run/reservation IDs, and a disconnected deterministicghfixture that recorded dispatch intent and injected one zero-success failure. No GitHub API, production repository dispatch, queue, gate, schedule, comment, label, workflow, or dashboard mutation occurred.Command/environment: Crabbox provider
local-container, imagemcr.microsoft.com/playwright:v1.60.0-noble, checked-indocs/proof/adaptive-hot-review/run-proof.sh. After two Crabbox rsync handshakes failed before copying any bytes, the clean exact-head checkout was seeded into the same provider from a verified read-only Git bundle and the proof ran through Crabbox with--no-sync; the script performed its own frozen install/build without Actions hydration.Observed result:
RESULT: PASS.Trace/artifacts: lease
cbx_0d5a25303ead(tidal-crayfish), providerlocal-container, image above, exit 0. The local proof package containsproof-summary.json,runtime-transcript.md, focused TAP, replay JSON, and Wrangler logs; it contains synthetic values only and is not published as a repository artifact. The lease was stopped after artifacts were copied.Limits: this proves the delivered activation machinery while deliberately leaving production activation disconnected. It does not demonstrate live GitHub repository dispatch, real fleet yield, live quota consumption, or a production canary. Those are operational rollout observations after a separately approved activation, not development-time proof authorization.
Review closeout
Accepted findings fixed during iterative dirty and committed Codex reviews include fairness/cursor starvation, global-token accounting, probe isolation, shadow cursor fencing, one-shot reservations, dispatch-abort recovery, partial-dispatch duplicate prevention, direct-write exclusion during active reservations, hot-only adaptive configuration parsing, canonical retention wiring, kill-switch bypass of malformed inactive controls, independent legacy cursor persistence across shadow comparison failures, fail-open empty-plan observation without default-branch or queue-capability hard dependencies, canonical operator-documentation links, exact snapshot parsing, policy-version observation isolation/order, excluding observation fallback from readiness, deterministically bounding the public observation snapshot to its advertised 100 partitions, separating that public cap from a signed current-policy hot-lane allocator view covering the eligible fleet, strict public decision projection, aligning the documented 25% cap with the over-offered candidate budget, running the actual fanout command through its Worker settlement boundary, preserving measured review runtime end to end instead of replacing it with lease elapsed time, and reserving the final constrained-cycle slot for ordinary demand before overdue, novelty, or probe rounds can exhaust it. The final committed review reports no discrete actionable correctness issues.
The full local ClawSweeper range loop found and drove fixes for fallback readiness, the public observation cap, strict decision sanitization, and the documentation mismatch. The final current-base artifact reviewed
c145b4599285969b43e40d0816071893d5090501...ceb36659a534ad77f88f0573b68e4d7f3f602563, reportedreal_behavior_proof_status: sufficient, zero review comments, andOverall correctness: patch is correct. Its B/A/B overall/proof/patch rating reflects the broad central-automation scope and the still-required maintainer acceptance of the documented at-most-once settlement tradeoff, not an actionable correctness finding. Hosted@clawsweeper re-reviewwill be requested only after the exact final body and head are on the draft PR.Bay impact
Observer-only. The existing queue/status projection can display the bounded adaptive snapshot, but this PR adds no Bay controls, actions, activation path, or mutation surface.
Risks and maintainer decisions
Related work