-
Notifications
You must be signed in to change notification settings - Fork 0
⚡ perf: cache historical github year in review data #518
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,12 +89,12 @@ type GitHubCommit = { | |
|
|
||
|
|
||
|
|
||
| async function graphql<T>(query: string, token: string, variables: Record<string, unknown>): Promise<T> { | ||
| async function graphql<T>(query: string, token: string, variables: Record<string, unknown>, cacheOpt: RequestCache = "no-store"): Promise<T> { | ||
| const res = await fetch(GITHUB_GRAPHQL, { | ||
| method: "POST", | ||
| headers: headers(token), | ||
| body: JSON.stringify({ query, variables }), | ||
| cache: "no-store", | ||
| cache: cacheOpt, | ||
| }); | ||
|
|
||
| if (res.status === 403) { | ||
|
|
@@ -144,7 +144,8 @@ async function fetchCommitDatesForTopRepos( | |
| token: string, | ||
| fromIso: string, | ||
| toIso: string, | ||
| repositories?: ContributionsByRepoNode[] | ||
| repositories?: ContributionsByRepoNode[], | ||
| cacheOpt?: RequestCache | ||
| ): Promise<string[]> { | ||
| const candidates = (repositories || []) | ||
| .filter((repo) => repo.contributions.totalCount > 0) | ||
|
|
@@ -193,7 +194,7 @@ async function fetchCommitDatesForTopRepos( | |
| }`; | ||
|
|
||
| try { | ||
| const response = await graphql<Record<string, unknown>>(query, token, variables); | ||
| const response = await graphql<Record<string, unknown>>(query, token, variables, cacheOpt); | ||
| const dates: string[] = []; | ||
|
|
||
| for (let i = 0; i < candidates.length; i++) { | ||
|
|
@@ -252,12 +253,15 @@ export async function fetchYearInReviewData(username: string, year: number, toke | |
| const to = new Date(Date.UTC(year, 11, 31, 23, 59, 59)); | ||
|
|
||
| try { | ||
| const currentYear = new Date().getFullYear(); | ||
| const cacheOpt: RequestCache = year < currentYear ? "force-cache" : "no-store"; | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 過去年でもコミットの追加・削除、リポジトリの公開範囲変更、private contribution の表示設定変更などで GitHub の集計は更新されますが、この変更は GraphQL と REST の応答を再検証期限なしで Knowledge Base Used: Year in Review Prompt To Fix With AIThis is a comment left during a code review.
Path: src/lib/githubYearInReview.ts
Line: 258
Comment:
**過去年キャッシュが再検証されない**
過去年でもコミットの追加・削除、リポジトリの公開範囲変更、private contribution の表示設定変更などで GitHub の集計は更新されますが、この変更は GraphQL と REST の応答を再検証期限なしで `force-cache` に固定します。そのため、最初に保存された contribution totals、top repository、most-active hour、heatmap が GitHub の最新状態と一致しなくなり、キャッシュが外部要因で消えるまで古い年次レビューが返り続けます。
**Knowledge Base Used:** [Year in Review](https://app.greptile.com/hiroki-org/-/custom-context/knowledge-base/hiroki-org/github-user-summary/-/docs/year-in-review.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+256
to
+258
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2. Timezone year mismatch caching fetchYearInReviewData (and fetchCommitActivityHeatmap) uses new Date().getFullYear() (server local timezone) to choose "force-cache" vs "no-store", but the API route validates requested years against getUTCFullYear() and the from/to bounds are built with Date.UTC. Around New Year on servers ahead of UTC, this can incorrectly treat the current UTC year as historical and cache it, serving stale results for that window. Agent Prompt
|
||
| const response = await graphql<YearInReviewResponse>(YEAR_IN_REVIEW_QUERY, token, { | ||
| login: username, | ||
| from: from.toISOString(), | ||
| to: to.toISOString(), | ||
| maxRepositories: 10, | ||
| }); | ||
| }, cacheOpt); | ||
|
Comment on lines
255
to
+264
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. cacheopt behavior untested This PR changes cache behavior by switching GitHub requests to force-cache for historical years,
but no test asserts the new fetch(..., { cache: ... }) behavior for past vs current year. Without
explicit assertions, regressions (e.g., always using no-store) could slip in unnoticed.
Agent Prompt
|
||
|
|
||
| if (!response.user) { | ||
| throw new UserNotFoundError(username); | ||
|
|
@@ -270,7 +274,8 @@ export async function fetchYearInReviewData(username: string, year: number, toke | |
| token, | ||
| from.toISOString(), | ||
| to.toISOString(), | ||
| collection.commitContributionsByRepository | ||
| collection.commitContributionsByRepository, | ||
| cacheOpt | ||
| ); | ||
|
|
||
| const commitDates = await commitDatesPromise; | ||
|
|
@@ -292,12 +297,15 @@ export async function fetchCommitActivityHeatmap(username: string, year: number, | |
| const from = new Date(Date.UTC(year, 0, 1, 0, 0, 0)); | ||
| const to = new Date(Date.UTC(year, 11, 31, 23, 59, 59)); | ||
|
|
||
| const currentYear = new Date().getFullYear(); | ||
| const cacheOpt: RequestCache = year < currentYear ? "force-cache" : "no-store"; | ||
|
|
||
| const reposResponse = await graphql<YearInReviewResponse>(YEAR_IN_REVIEW_QUERY, token, { | ||
| login: username, | ||
| from: from.toISOString(), | ||
| to: to.toISOString(), | ||
| maxRepositories: 10, | ||
| }); | ||
| }, cacheOpt); | ||
|
|
||
| if (!reposResponse.user) { | ||
| throw new UserNotFoundError(username); | ||
|
|
@@ -316,7 +324,7 @@ export async function fetchCommitActivityHeatmap(username: string, year: number, | |
| url.searchParams.set("until", to.toISOString()); | ||
| url.searchParams.set("per_page", "100"); | ||
|
|
||
| const res = await fetch(url.toString(), { headers: headers(token), cache: "no-store" }); | ||
| const res = await fetch(url.toString(), { headers: headers(token), cache: cacheOpt }); | ||
| if (res.status === 403) { | ||
| handleRateLimit(res); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
サーバーのタイムゾーンが UTC より進んでいる場合、年越し直後は API とクライアントが
getUTCFullYear()で選ぶ現在年を、ここではgetFullYear()により過去年と判定します。その時間帯だけ更新中の年次データがforce-cacheの対象になり、不完全な集計がキャッシュされるため、検索期間や API 検証と同じ UTC 基準を使用してください。Knowledge Base Used: Year in Review
Prompt To Fix With AI