Skip to content

fix(scheduler): prioritize canonical coverage gaps - #960

Merged
steipete merged 1 commit into
mainfrom
fix/untracked-dispatch-priority
Jul 30, 2026
Merged

fix(scheduler): prioritize canonical coverage gaps#960
steipete merged 1 commit into
mainfrom
fix/untracked-dispatch-priority

Conversation

@steipete

Copy link
Copy Markdown
Contributor

What Problem This Solves

The scheduler was using two different meanings of “tracked.” The live coverage endpoint counts only modern canonical Worker tuples, while planner hydration contains both those tuples and older imported backfill reports. PR #956 prioritized only items with no hydrated report at all, so a legacy backfill report incorrectly yielded to canonical re-reviews even though /api/review-coverage still counted that item as untracked_open.

This is why completed reviews barely moved coverage: the ordering fix ran, but against the wrong identity boundary.

Production Evidence

Recent planner run 30542575714, on the #958 merge commit, hydrated canonical Worker state at revision 90804 and scanned 5,122 due openclaw/openclaw candidates. Its 51 selected candidates contained:

  • 2 candidates with no prior report
  • 49 candidates with lastReviewedAt, including records reviewed in June

At the same time, GET /api/review-coverage reported 2,651 untracked_open for openclaw/openclaw and 2,749 fleet-wide. The planner therefore collapsed almost the entire operational coverage gap into the tracked cohort before selectDueCandidates ordered it.

The queue was not the blocker. Its live review lane had 122 free slots and the normal-backfill token bucket held its full 78-token burst. No guaranteed-claim floor can help candidates that never reach enqueue with the right priority.

Fix

  • During Worker hydration, page the already-deployed authenticated canonical items listing and persist the exact canonical item identities in the worker-records manifest.
  • Make fleet fanout calculate untracked backlog from those canonical identities instead of counting every hydrated legacy report file.
  • Make target planners classify and prioritize missing-canonical-tuple items even when a legacy report remains available as review context.
  • Treat missing canonical coverage as immediately due, emit coverageTracked in plan telemetry, and keep every coverage-untracked candidate ahead of canonical refreshes.
  • Require the manifest explicitly in production fanout and planning workflow paths while preserving local CLI fallback behavior.

No Cloudflare Worker redeploy or storage migration is required: the authenticated canonical record-list endpoint is already deployed. The workflow/planner change has no production effect until this PR reaches main.

Proof

  • Focused planner, enqueue, fanout, manifest, hydration request, and workflow tests: 135/135 passed.
  • Regression: with 3,000 legacy-backed but canonical-untracked items and 20 older canonical refreshes, the planner selected 128/128 untracked candidates and all 128 reached mocked queue admission.
  • pnpm run format:check and pnpm run lint: passed.
  • pnpm run check: static checks, all builds, lint, changed-surface coverage, and coverage thresholds passed; the full suite passed 2,797 tests, skipped 9, and failed only the five untouched macOS containment fixtures that import Linux libc.capset.
  • ~/.claude/skills/autoreview/scripts/autoreview --mode local: clean, no accepted/actionable findings, correctness 0.98.
  • ~/.claude/skills/autoreview/scripts/autoreview --mode branch --base origin/main: clean, no accepted/actionable findings, correctness 0.93.

Expected Burn

The observed pre-fix movement was 5 items in 25 minutes, about 12 untracked items/hour. After this fix, the 390/hour normal-backfill lane becomes the admission ceiling. With 10-minute fanout, smaller-repository fairness reservations, and openclaw/openclaw holding about 96% of the current fleet gap, the expected steady review burn for that repository is roughly 300–325 canonical-untracked items/hour, with the banked burst improving the first cycle.

The public counter moves only after durable publication. The live publication lane was publishing about 208 items/hour, so dashboard-visible burn should initially be about 200/hour until that lane catches up, then approach the roughly 300/hour review rate if publication capacity is no longer the limiter.

@steipete
steipete requested a review from a team as a code owner July 30, 2026 13:29
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Jul 30, 2026
@clawsweeper

clawsweeper Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 30, 2026, 9:33 AM ET / 13:33 UTC.

ClawSweeper review

What this changes

The PR makes scheduled review planning use canonical Worker tuple identities rather than legacy report files when prioritizing open coverage gaps.

Merge readiness

Blocked until real behavior proof from a real setup is added - 5 items remain

Keep this PR open: the canonical-identity design is a focused follow-up to the merged scheduler work, but the branch still needs after-fix real workflow evidence and should not include a release-owned changelog edit. Likely related people: steipete is the recent scheduler contributor behind the two merged prerequisite changes (medium confidence).

Priority: P2
Reviewed head: 9cd7b6b46d6064d3bf69bd8f2a2d99561d074345

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The patch is focused and regression-tested, but real after-fix automation proof is still missing.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: The PR body provides focused tests, mocked queue admission, and historical production observations, but it does not show an after-fix run of the changed manifest-to-planner workflow path. Add redacted live output, logs, or a terminal recording; redact secrets, private endpoints, and identifiers. After updating the PR body, a fresh ClawSweeper review should run, or a maintainer can request one with @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR body provides focused tests, mocked queue admission, and historical production observations, but it does not show an after-fix run of the changed manifest-to-planner workflow path. Add redacted live output, logs, or a terminal recording; redact secrets, private endpoints, and identifiers. After updating the PR body, a fresh ClawSweeper review should run, or a maintainer can request one with @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 6 items Canonical identity classification: The proposed planner path accepts a canonical coverage-ID set, treats items absent from that set as immediately due even when a legacy report exists, and records the classification in selection telemetry.
Coverage-first scheduler ordering: The scheduler changes its first-review cohort from records with no report to candidates explicitly marked coverage-untracked, preserving legacy reports as context while giving missing canonical tuples priority.
Workflow dependency: Both normal fanout and target planning now receive the worker-records manifest path, while the manifest reader rejects absent or unsupported canonical coverage data. That makes runtime proof of the hydrated artifact and planning path important before merge.
Findings 1 actionable finding [P3] Remove the release-owned changelog entry
Security None None.

How this fits together

The scheduler hydrates durable Worker review records, selects due GitHub issues and pull requests, and sends the chosen candidates into the exact-review queue. This change aligns fanout and per-repository selection with the coverage endpoint’s canonical identity boundary so legacy report context does not suppress first canonical reviews.

flowchart LR
  A[Worker canonical item tuples] --> B[Hydrated records manifest]
  C[Legacy backfill reports] --> D[Planner review context]
  B --> E[Coverage-tracked classification]
  D --> E
  F[Live open GitHub items] --> E
  E --> G[Priority candidate selection]
  G --> H[Exact-review queue]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR body provides focused tests, mocked queue admission, and historical production observations, but it does not show an after-fix run of the changed manifest-to-planner workflow path. Add redacted live output, logs, or a terminal recording; redact secrets, private endpoints, and identifiers. After updating the PR body, a fresh ClawSweeper review should run, or a maintainer can request one with @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Remove the release-owned changelog entry (P3) - Keep this release-note summary in the PR body or commit context instead. Target policy reserves CHANGELOG.md for release ownership, so this normal scheduler PR should not modify it.
  • Resolve merge risk (P1) - Merging changes the production scheduling workflow to require a hydrated canonical-coverage manifest; a missing, malformed, or unexpectedly incomplete manifest can halt or mis-prioritize normal review planning despite unit coverage.
  • Resolve merge risk (P1) - The PR body shows tests, mocked queue admission, and pre-fix production observations, but not an after-fix execution of the manifest-to-planner path in a real workflow environment.
  • Complete next step (P2) - A narrow mechanical repair can remove the release-owned changelog line, while the contributor remains responsible for supplying the required real behavior proof.

Findings

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:12
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch scope 14 files affected; 440 added, 24 removed The change spans Worker hydration, scheduler selection, fanout, workflow wiring, docs, and focused tests, so the end-to-end artifact handoff matters more than a unit-only pass.
Automation wiring 2 scheduled workflow invocations changed Both fanout and target planning now depend on the canonical-coverage manifest.

Merge-risk options

Maintainer options:

  1. Prove the production planner path (recommended)
    Run the current PR head through a redacted real workflow or equivalent authenticated staging path that materializes canonical identities and shows the resulting coverage-untracked plan and admissions before merging.
  2. Pause for evidence
    Keep the PR open if a realistic workflow run cannot be performed yet, because the required manifest is now part of scheduled review automation.

Technical review

Best possible solution:

Remove the release-owned changelog line and add redacted after-fix workflow evidence showing a hydrated canonical manifest, coverage-untracked selection ahead of canonical refreshes, and queue admission for the selected candidates.

Do we have a high-confidence way to reproduce the issue?

Unclear for a live current-head reproduction: the supplied production observations establish the pre-fix mismatch, and the proposed source/test scenario is reproducible, but no after-fix workflow run was provided.

Is this the best way to solve the issue?

Yes in design: carrying exact canonical identities through hydration, fanout, and planning is narrower and less ambiguous than inferring coverage from legacy report files. The real workflow handoff still needs proof before this is the merge-ready solution.

Full review comments:

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:12
    Keep this release-note summary in the PR body or commit context instead. Target policy reserves CHANGELOG.md for release ownership, so this normal scheduler PR should not modify it.
    Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.79

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against fd8f2acf8a31.

Labels

Label changes:

  • add P2: This is a bounded scheduler correction affecting review coverage throughput and queue selection, with no evidence of an active security or data-loss emergency.
  • add merge-risk: 🚨 automation: The PR changes scheduled workflow arguments and makes production planner execution depend on a hydrated manifest artifact.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides focused tests, mocked queue admission, and historical production observations, but it does not show an after-fix run of the changed manifest-to-planner workflow path. Add redacted live output, logs, or a terminal recording; redact secrets, private endpoints, and identifiers. After updating the PR body, a fresh ClawSweeper review should run, or a maintainer can request one with @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This is a bounded scheduler correction affecting review coverage throughput and queue selection, with no evidence of an active security or data-loss emergency.
  • merge-risk: 🚨 automation: The PR changes scheduled workflow arguments and makes production planner execution depend on a hydrated manifest artifact.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides focused tests, mocked queue admission, and historical production observations, but it does not show an after-fix run of the changed manifest-to-planner workflow path. Add redacted live output, logs, or a terminal recording; redact secrets, private endpoints, and identifiers. After updating the PR body, a fresh ClawSweeper review should run, or a maintainer can request one with @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

Acceptance criteria:

  • [P1] git diff --check.

What I checked:

  • Canonical identity classification: The proposed planner path accepts a canonical coverage-ID set, treats items absent from that set as immediately due even when a legacy report exists, and records the classification in selection telemetry. (src/clawsweeper.ts:8487, 9cd7b6b46d60)
  • Coverage-first scheduler ordering: The scheduler changes its first-review cohort from records with no report to candidates explicitly marked coverage-untracked, preserving legacy reports as context while giving missing canonical tuples priority. (src/scheduler-policy.ts:335, 9cd7b6b46d60)
  • Workflow dependency: Both normal fanout and target planning now receive the worker-records manifest path, while the manifest reader rejects absent or unsupported canonical coverage data. That makes runtime proof of the hydrated artifact and planning path important before merge. (.github/workflows/sweep.yml:2525, 9cd7b6b46d60)
  • Regression coverage: The branch adds focused tests for legacy-backed canonical-untracked candidates winning all 128 planner and mocked queue-admission slots, manifest validation, fanout counts, and workflow argument propagation. (test/repair/scheduled-review-enqueue.test.ts:6, 9cd7b6b46d60)
  • Recent feature history: The supplied related-item history shows the earlier coverage-ordering and candidate-capacity work merged as commits d199482 and c5c8ddf; this PR addresses the remaining mismatch between legacy hydrated records and canonical coverage tuples. (src/scheduler-policy.ts:335, c5c8ddf0d2e5)
  • Repository review policy: The supplied target AGENTS.md treats CHANGELOG.md as release-owned and directs normal PRs to keep release-note context in the PR body or commits rather than editing that file. (CHANGELOG.md:12, 9cd7b6b46d60)

Likely related people:

  • steipete: The supplied related history identifies steipete as the author of the two immediately preceding merged scheduler changes that this PR refines. (role: recent scheduler contributor; confidence: medium; commits: d199482415e0, c5c8ddf0d2e5; files: src/scheduler-policy.ts, src/repair/target-fanout.ts, .github/workflows/sweep.yml)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Remove the release-owned CHANGELOG.md entry.
  • Add redacted after-fix evidence from canonical-manifest hydration through scheduler selection and queue admission.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete merged commit 9e464d6 into main Jul 30, 2026
19 checks passed
@steipete
steipete deleted the fix/untracked-dispatch-priority branch July 30, 2026 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant