Skip to content

Per-event comment acknowledgment + quota backoff ceiling - #170

Merged
asavs merged 1 commit into
mainfrom
feat/per-event-ack-and-short-quota-backoff
Jul 9, 2026
Merged

asavs merged 1 commit into
mainfrom
feat/per-event-ack-and-short-quota-backoff

Conversation

@asavs

@asavs asavs commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Two operational fixes from tonight's PR mog-template#219 incident.

Per-event acknowledgment

The eyes/confused reactions only ever landed on the PR body, and reactions are idempotent per user+emoji — so after the first tick the daemon could never signal I saw your new comment/commit. Now every review-start (👀) and quota-stuck (😕) signal also reacts on the PR's newest issue comment via issues/comments/<id>/reactions. A new comment is a fresh reaction target, so acknowledgment finally tracks activity. Best-effort, logged, and a comment-less PR keeps the old body-only behavior.

Quota backoff ceiling

Tonight the daemon sat in a ~1h backoff while quota was already back: the 429 said Resets in 20h26m23s, which (a) was flat wrong — quota recovered within the hour — and (b) never matched the XmXs-only parser, silently falling through to the 1-hour default. Since the API's reset estimates are unreliable in both directions and a premature retry costs exactly one failed agy call, every backoff is now clamped to REVIEWER_AGY_QUOTA_MAX_BACKOFF (default 600s); the default backoff drops 3600→600s and padding 300→60s. A recovered quota is discovered within ~10 minutes instead of an hour-plus.

Tests

Suite green: 570 assertions (was 566). New coverage: clamp on huge retryDelayMs, clamp on the hours-scale reset format, comment-level confused reaction in the quota loop fixture (endpoint + log line), and empty-comment-list no-ops in the escalation/research fixtures.

…f ceiling

Reactions on the PR body are idempotent per user+emoji, so the daemon
could never signal 'I saw your new comment' -- eyes/confused now also
land on the PR's newest issue comment (fresh comment = fresh reaction
target); PRs without comments keep the body-only behavior.

Quota backoffs are now clamped to REVIEWER_AGY_QUOTA_MAX_BACKOFF
(default 600s) whatever the 429 body claims: the API's reset estimates
have been observed wrong in both directions (a 'Resets in 20h26m'
quota came back within the hour), and the hours-scale reset format
never matched the m/s parser anyway, silently falling to the 1-hour
default. Default backoff drops 3600->600s and padding 300->60s; a
recovered quota is now discovered within ~10 minutes at the cost of
one failed agy call per retry.
@asavs
asavs merged commit 661b4e7 into main Jul 9, 2026
1 check passed
@asavs
asavs deleted the feat/per-event-ack-and-short-quota-backoff branch July 9, 2026 23:01
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.

1 participant