Skip to content

fix(code-reviews): bound cron pending drain#3396

Open
alex-alecu wants to merge 9 commits into
mainfrom
fix/code-review-cron-pending-cutoff
Open

fix(code-reviews): bound cron pending drain#3396
alex-alecu wants to merge 9 commits into
mainfrom
fix/code-review-cron-pending-cutoff

Conversation

@alex-alecu
Copy link
Copy Markdown
Contributor

@alex-alecu alex-alecu commented May 21, 2026

Summary

Followup to #3357

  • Bound the cron pending code-review drain to pending reviews created between 60 and 75 minutes ago, matching the plan's main goal while preserving direct/manual dispatch as unbounded.
  • Threaded the same created_at recovery window through owner discovery and per-owner reservation so cron candidate selection and claiming agree.
  • Removed the unrelated retry-budget schema, migration, release-claim, and fixture changes so this PR has no database migration or manual retry semantic change.

Reviewer Notes

  • Stale queued recovery remains independent of the pending created_at window.
  • The cron window is intentionally narrow and inclusive: reviews aged 60 through 75 minutes are eligible.
  • Direct calls to tryDispatchPendingReviews(owner) still consider all pending reviews.

@kilo-code-bot
Copy link
Copy Markdown
Contributor

kilo-code-bot Bot commented May 21, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The only change since the previous review (3afe503) is b9e9973 — a cosmetic fix removing a trailing newline from the generated migration journal. No logic, schema, or application code was modified; all previous findings remain resolved.

Files Reviewed (1 incremental file)
  • packages/db/src/migrations/meta/_journal.json — generated file; EOF whitespace fix only, no logic change

Reviewed by claude-4.6-sonnet-20260217 · 161,279 tokens

Review guidance: REVIEW.md from base branch main

Comment thread apps/web/src/lib/code-reviews/dispatch/dispatch-pending-code-review-owners.ts Outdated
Comment thread apps/web/src/lib/code-reviews/dispatch/dispatch-constants.ts Outdated
Restore the cron-only pending cutoff that was removed when fixing the
manual retry stranding case. The previous fix made the cron scan
unbounded, scanning every old pending row; this brings back the bound
but keys it on `updated_at` instead of `created_at` so manual retries
(which refresh `updated_at` while preserving `created_at`) remain
eligible for the next cron pass when immediate dispatch can't run.

- Add CRON_PENDING_CODE_REVIEW_MAX_IDLE_HOURS / cronPendingCodeReviewUpdatedAfterSql.
- Thread an optional pendingUpdatedAfter through reconsiderableCodeReviewWorkCondition,
  listDispatchableCodeReviewOwnerCandidates, and tryDispatchPendingReviews.
- Cron drain (dispatchPendingCodeReviewOwners) supplies the cutoff for
  both owner discovery and per-owner drain; direct webhook/status/manual
  paths keep the unbounded behavior.
- Stale queued recovery is unaffected by the new gate.
- Tests cover the bounded cron path, the unbounded direct path, and the
  retry-refreshes-updated_at invariant.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants