fix(github-poller): drop page-1 ETag sentinel on paginated endpoints#81
Conversation
The poller's "send cached page-1 ETag, on 304 skip the walk" trick is only valid for endpoints GitHub sorts newest-first. For oldest-first endpoints (stargazers default, forks?sort=oldest), once page 1 fills to 100 entries it becomes immutable and the ETag freezes -- the poller then treats every subsequent 304 as "nothing changed" and the cache silently strands all entries on pages 2+. Visible failure: /stats stars graph stuck at 100 in production while the repo grew past 110. Fix: never send If-None-Match on paginated endpoints; always walk all pages. Non-paginated `repo-summary` keeps the ETag round-trip (no pagination wrinkle there). Cost: ~14 extra GitHub req/hr across the 6 paginated endpoints, well under the 5000 req/hr authenticated cap. Verified with in-process mock-fetch simulation: pre-fix with a stale page-1 ETag fed in -> touchGithubCacheRow only (cache stuck at 100); post-fix -> upsertGithubCacheRow with 114 entries (matches live).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c71ac48c3a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const firstUrl = `${base}?${cfg.query}${cfg.query ? '&' : ''}page=1&per_page=${PER_PAGE}`; | ||
|
|
||
| const firstResp = await fetchOnePage(firstUrl, cfg.accept, existingEtag, fetchImpl); | ||
| const firstResp = await fetchOnePage(firstUrl, cfg.accept, null, fetchImpl); |
There was a problem hiding this comment.
Passing null for If-None-Match on paginated endpoints forces full polling every 5 minutes, which guarantees at least 84 unauthenticated REST calls/hour (6 paginated endpoints + repo-summary across 12 ticks) before extra pages are counted. In environments without GITHUB_TOKEN, this deterministically exceeds GitHub’s 60/hr unauthenticated limit, after which the page=1 fetches return non-200 and this function skips cache updates, leaving stats stale. This is a behavior regression from the previous conditional-request path for tokenless deployments.
Useful? React with 👍 / 👎.
Addresses Codex P1 on PR #81: the previous fix dropped If-None-Match on all paginated endpoints, which would have pushed tokenless deployments past the 60 req/hr unauthenticated cap (7 endpoints x 12 ticks/hr = 84 baseline first-page requests, all 200s). GitHub's rate-limit policy doesn't count 304 responses against the primary limit, so the original ETag round-trip was effectively free for newest-first endpoints (commits, issues, pulls, releases) -- and it's still correct there because new entries land on page 1, so page 1's ETag is a valid whole-dataset freshness sentinel. Only `stars` (stargazers default sort = oldest-first) and `forks?sort=oldest` need the always-walk treatment, because page 1 becomes immutable once it fills to 100 entries. Marked those two with `oldestFirst: true` in the ENDPOINTS config; pollEndpoint reads the flag and only sends If-None-Match when it's false/absent. Net cost vs. pre-fix: ~36 extra req/hr (stars walks 2 pages currently, forks walks 1) -- still inside the 60 req/hr per-IP unauthenticated cap and trivial against the 5000 req/hr authenticated cap. Newest-first endpoints retain their original tokenless-friendly behavior.
|
@codex review — addressed the P1 in 21d04b8. Only the oldest-first endpoints (stars + forks?sort=oldest) now opt out of the ETag sentinel via |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
/statsstars graph stuck at 100 in prod while the repo grew past 110.showcase/server/src/telemetry/github-poller.js— for endpoints GitHub sorts newest-first (commits, releases, etc.) the trick works because page 1 mutates on every new entry; for oldest-first endpoints (stargazers default,forks?sort=oldest) page 1 freezes at 100 entries forever and the ETag never changes.If-None-Matchon paginated endpoints; always re-walk all pages. Non-paginatedrepo-summarykeeps the ETag round-trip (no pagination wrinkle there). Cost: ~14 extra GitHub req/hr across the 6 paginated endpoints, well under the 5000 req/hr authenticated cap.forks(currently 9 forks; would have hit the moment count crossed 100).Investigation
Full root-cause report in
.planning/debug/stars-graph-stuck-at-100.md(not part of this PR — internal debug artifact).Key evidence:
curl .../api/public-stats/github/stars | jq length→100exactlycurl https://api.github.com/repos/LakshmanTurlapati/FSB/stargazers?page=1&per_page=100→ 100 entries with identical first/last bookends to the stale cachecurl .../api/public-stats/github/stargazers?page=2&per_page=100→ 14 entries (the missing ones)touchGithubCacheRowonly (cache stays at 100); post-fix →upsertGithubCacheRowwith 114 entries.Test plan
curl -s https://full-selfbrowsing.com/api/public-stats/github/stars | jq lengthreturns ≥ livestargazers_count(currently 114) and continues to grow as new stars come inrepo-summaryendpoint unchanged:curl -s https://full-selfbrowsing.com/api/public-stats/github/repo-summary | jq .stargazers_countstill matches live/statspage in browser shows current star count (not 100)