Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 89 additions & 0 deletions src/lib/__tests__/githubYearInReview.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,95 @@ describe("fetchYearInReviewData success paths", () => {
});
});


describe("fetchYearInReviewData caching logic", () => {
const currentYear = new Date().getFullYear();

beforeEach(() => {
mockFetch.mockResolvedValue(jsonResponse({
data: {
user: {
id: "U_123",
contributionsCollection: {
totalCommitContributions: 10,
totalPullRequestContributions: 5,
totalIssueContributions: 2,
totalPullRequestReviewContributions: 1,
contributionCalendar: {
totalContributions: 18,
weeks: []
},
commitContributionsByRepository: [],
pullRequestContributionsByRepository: [],
issueContributionsByRepository: []
}
}
}
}));
});

it("should use cache: 'no-store' for the current year", async () => {
await fetchYearInReviewData("testuser", currentYear, "mock-token");
expect(mockFetch).toHaveBeenCalledWith("https://api.github.com/graphql", expect.objectContaining({
cache: "no-store"
}));
});

it("should use cache: 'force-cache' for a past year", async () => {
await fetchYearInReviewData("testuser", currentYear - 1, "mock-token");
expect(mockFetch).toHaveBeenCalledWith("https://api.github.com/graphql", expect.objectContaining({
cache: "force-cache"
}));
});
});

describe("fetchCommitActivityHeatmap caching logic", () => {
const currentYear = new Date().getFullYear();

beforeEach(() => {
// 1st call for repos
mockFetch.mockResolvedValueOnce(jsonResponse({
data: {
user: {
id: "U_123",
contributionsCollection: {
commitContributionsByRepository: [{
repository: { owner: { login: "own" }, name: "repo" },
contributions: { totalCount: 1 }
}]
}
}
}
}));
// 2nd call for commits
mockFetch.mockResolvedValueOnce(jsonResponse([{
commit: { author: { date: "2023-01-01T12:00:00Z" } }
}]));
});

it("should use cache: 'no-store' for the current year", async () => {
await fetchCommitActivityHeatmap("testuser", currentYear, "mock-token");
expect(mockFetch).toHaveBeenCalledWith("https://api.github.com/graphql", expect.objectContaining({
cache: "no-store"
}));
// URL will have the parameters
expect(mockFetch).toHaveBeenCalledWith(expect.stringContaining("https://api.github.com/repos/own/repo/commits"), expect.objectContaining({
cache: "no-store"
}));
});

it("should use cache: 'force-cache' for a past year", async () => {
await fetchCommitActivityHeatmap("testuser", currentYear - 1, "mock-token");
expect(mockFetch).toHaveBeenCalledWith("https://api.github.com/graphql", expect.objectContaining({
cache: "force-cache"
}));
expect(mockFetch).toHaveBeenCalledWith(expect.stringContaining("https://api.github.com/repos/own/repo/commits"), expect.objectContaining({
cache: "force-cache"
}));
});
});


describe("fetchCommitActivityHeatmap", () => {
it("successfully fetches and builds commit activity heatmap", async () => {
mockFetch.mockImplementation((url: string | URL | Request) => {
Expand Down
24 changes: 16 additions & 8 deletions src/lib/githubYearInReview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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++) {
Expand Down Expand Up @@ -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";
Comment on lines +256 to +257

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 現在年判定が UTC と不一致

サーバーのタイムゾーンが UTC より進んでいる場合、年越し直後は API とクライアントが getUTCFullYear() で選ぶ現在年を、ここでは getFullYear() により過去年と判定します。その時間帯だけ更新中の年次データが force-cache の対象になり、不完全な集計がキャッシュされるため、検索期間や API 検証と同じ UTC 基準を使用してください。

Knowledge Base Used: Year in Review

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/githubYearInReview.ts
Line: 256-257

Comment:
**現在年判定が UTC と不一致**

サーバーのタイムゾーンが UTC より進んでいる場合、年越し直後は API とクライアントが `getUTCFullYear()` で選ぶ現在年を、ここでは `getFullYear()` により過去年と判定します。その時間帯だけ更新中の年次データが `force-cache` の対象になり、不完全な集計がキャッシュされるため、検索期間や API 検証と同じ UTC 基準を使用してください。

**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.


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 過去年キャッシュが再検証されない

過去年でもコミットの追加・削除、リポジトリの公開範囲変更、private contribution の表示設定変更などで GitHub の集計は更新されますが、この変更は GraphQL と REST の応答を再検証期限なしで force-cache に固定します。そのため、最初に保存された contribution totals、top repository、most-active hour、heatmap が GitHub の最新状態と一致しなくなり、キャッシュが外部要因で消えるまで古い年次レビューが返り続けます。

Knowledge Base Used: Year in Review

Prompt To Fix With AI
This 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Timezone year mismatch caching 🐞 Bug ≡ Correctness

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
### Issue description
The caching decision for year-in-review requests uses `new Date().getFullYear()` (local timezone), while the rest of the system uses UTC year semantics (`getUTCFullYear()` and `Date.UTC(...)`). This mismatch can cause the current UTC year to be treated as “historical” and cached around New Year depending on server timezone.

### Issue Context
- The route handler selects/validates `year` using `getUTCFullYear()`.
- The data query window is constructed using `Date.UTC(...)`.
- The caching decision should follow the same UTC definition of “current year”.

### Fix
Replace `new Date().getFullYear()` with `new Date().getUTCFullYear()` in both call sites (or centralize into a small helper to avoid duplication).

### Fix Focus Areas
- src/lib/githubYearInReview.ts[252-264]
- src/lib/githubYearInReview.ts[292-308]
- src/app/api/dashboard/year/route.ts[18-22]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. cacheopt behavior untested 📘 Rule violation ▣ Testability

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
## Issue description
Caching behavior was changed to use `force-cache` for historical years and `no-store` for the current year, but tests do not assert that the correct `fetch` `cache` option is used.

## Issue Context
`fetchYearInReviewData` and `fetchCommitActivityHeatmap` now compute `cacheOpt` based on `year < new Date().getFullYear()` and pass it into the GraphQL helper / REST `fetch` call. Existing tests stub `fetch` but never validate the `cache` option in the request init.

## Fix Focus Areas
- src/lib/githubYearInReview.ts[255-308]
- src/lib/githubYearInReview.ts[327-327]
- src/lib/__tests__/githubYearInReview.test.ts[1-20]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


if (!response.user) {
throw new UserNotFoundError(username);
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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);
}
Expand Down
Loading