Skip to content

feat: back off PR polling for settled checks (adaptive TTL) #61

Description

@davidtaing

Problem

PR check polling re-fetches the full rollup on a fixed prCacheTTL (15s,
checks.go). gh pr list --json statusCheckRollup is a fresh API round-trip
every time — it returns the complete current rollup whether or not anything
changed, with no conditional/ETag/diff path. So a PR whose checks are settled
(all terminal — no IN_PROGRESS/QUEUED) gets re-fetched, byte-identical, every
15s for as long as it's in view. That's wasted round-trips: a settled PR won't
change until new check runs appear.

Proposed: adaptive (back-off) TTL

Keep the short 15s TTL while any check in the repo is pending; switch to a longer
TTL (e.g. 1–3 min) once everything is settled. After each fetch, inspect the
result: if any entry is StatePending, the repo is "hot" → short TTL; otherwise
"settled" → long TTL.

Structural note

The cache is per-repo, not per-PR — one gh pr list returns all of a repo's
open PRs in a single call (prCache keyed by dir, prCacheEntry). So back-off is
at repo granularity: a repo is "settled" only when every open PR in it has no
pending check. One pending PR keeps the whole repo on the short TTL (and refreshes
all of them — which is fine, it's one call).

The tradeoff (why hitl)

The reason to keep polling a settled PR is to notice when it stops being settled.
A longer TTL means new activity lags by up to that TTL:

  • a new commit → new pending runs: detected only on the next (slower) poll.
  • state can change with no local commit anyway — re-runs, re-requested
    reviews, external status contexts, a brand-new PR appearing, required-check
    config changes. So it's a longer TTL, never "cache forever."

So this trades some live-ness for fewer idle calls. Need a call on whether the
modest savings is worth the lag, and what the settled TTL should be (keep it
bounded — a couple of minutes — so the dashboard doesn't feel stale).

Worth it?

Polling is already heavily gated (filtered-only, maxPRPollProjects = 5, 15s
cache), and we're far from GitHub's 5000/hr authenticated limit — so the win is
modest. Filing for visibility rather than urgency.

Implementation notes

  • fetchPRChecks already has the fetched map in hand; compute a hot bool
    (any StatePending across the map) and store it on prCacheEntry alongside
    at. The freshness check picks prCacheTTL vs a new prCacheSettledTTL from
    that flag.
  • Pairs naturally with the GraphQL migration (feat: migrate PR fetch to GraphQL to expose required checks (+ review decision, PR number) #59), which reshapes the
    fetch/cache layer anyway — could land together or after.
  • Tests: a settled map keeps the entry fresh past 15s; a map with one pending
    entry does not. Mirror the table-driven style in checks_test.go.

Cut-line

Could ship as a pure afk change (pick a settled TTL, e.g. 90s, no new flag, no UI
change) if the tradeoff above is acceptable — the only "decision" is the number.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requesthitlNeeds a human (decision/design/review)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions