refactor!: rewrite and format code base#229
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis PR centralizes GitHub token resolution into a pure Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as Cards API
participant TokenSvc as getGitHubToken
participant GitHub
participant Analytics as GA (sendAnalytics)
Client->>API: GET /api/cards/<card>?username=...
API->>TokenSvc: getGitHubToken(0)
TokenSvc-->>API: token
API->>GitHub: fetch data (with token)
alt GitHub returns 401/403
API->>TokenSvc: getGitHubToken(nextIndex)
TokenSvc-->>API: rotated token
API->>GitHub: fetch data (with rotated token)
end
API->>API: render SVG (card generator with token)
API->>Analytics: sendAnalytics(event, params, headers)
API-->>Client: 200 + SVG (Cache-Control: CONST_CACHE_CONTROL)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (14)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
api/cards/productive-time.ts (1)
44-47: Don’tconsole.log(err)—can leak tokens/headers from HTTP client errors.
Line 45: axios errors often include request config/headers; logging the full object risks leakingAuthorization/token into logs. Logerr.messageand maybeerr.response?.statusinstead.Safer logging
} catch (err: any) { - console.log(err); + console.log(err?.message, err?.response?.status); res.send(getErrorMsgCard(err.message, theme)); }src/cards/productive-time-card.ts (1)
53-85: Guard against non-finiteutcOffsetto prevent invalid indexing.
In CLI usage,utcOffsetcan beNaN, which makesafterOffsetNaNand breakschartData[...] += 1.Proposed fix
const getProductiveTimeData = async function ( username: string, utcOffset: number, token: string ): Promise<Array<number>> { + const safeUtcOffset = Number.isFinite(utcOffset) ? utcOffset : 0; const until = new Date(); const since = new Date(); since.setFullYear(since.getFullYear() - 1); const productiveTime = await getProductiveTime(username, until.toISOString(), since.toISOString(), token); @@ for (const time of productiveTime.productiveDate) { const hour = new Date(time).getUTCHours(); // We use UTC+0 here - const afterOffset = adjustOffset(Number(hour) + Number(utcOffset), roundRobin); // Add offset to hour + const afterOffset = adjustOffset(Number(hour) + Number(safeUtcOffset), roundRobin); // Add offset to hoursrc/cards/stats-card.ts (1)
43-79: Defensive guard forcontributionYears[0]in Vercel labeling.
IfprofileDetails.contributionYearsis unexpectedly empty,${profileDetails.contributionYears[0]} Commits:will produceundefined Commits:(or worse if later refactors assume a string).Proposed tweak
- : { + : { index: 1, icon: Icon.COMMIT, - name: `${profileDetails.contributionYears[0]} Commits:`, + name: `${profileDetails.contributionYears[0] ?? 'Last year'} Commits:`, value: `${abbreviateNumber(totalCommitContributions, 1)}` },api/cards/most-commit-language.ts (1)
31-50: Fix token rotation starting point (don’t skipGITHUB_TOKEN_0/ avoid!).
Initializing withprocess.env.GITHUB_TOKEN!both (a) bypassesGITHUB_TOKEN_0if present and (b) can passundefineddownstream until a 401/403 happens. UsegetGitHubToken(0)as the single source of truth.Proposed patch
- let token = process.env.GITHUB_TOKEN!; - let tokenIndex = 0; + let tokenIndex = 0; + let token = getGitHubToken(tokenIndex); while (true) { try { const cardSVG = await getCommitsLanguageSVGWithThemeName(username, theme, excludeArr, token); await sendAnalytics('most-commit-language-card', {username, theme}); res.setHeader('Content-Type', 'image/svg+xml'); res.setHeader('Cache-Control', CONST_CACHE_CONTROL); res.send(cardSVG); return; } catch (err: any) { console.log(err.message); // We update github token and try again, until getNextGitHubToken throw an Error - if (err.response && (err.response.status === 403 || err.response.status === 401)) { + const status = err?.response?.status; + if (status === 403 || status === 401) { tokenIndex += 1; token = getGitHubToken(tokenIndex); } else { throw err; } } }
🤖 Fix all issues with AI agents
In @.github/workflows/manual-release.yml:
- Line 64: Update the GitHub Action reference for the release step from
softprops/action-gh-release@v1 to softprops/action-gh-release@v2 by changing the
uses line (uses: softprops/action-gh-release@v1 → uses:
softprops/action-gh-release@v2); after updating, run or validate the workflow to
ensure any v2 input/behavior changes are handled (adjust inputs or token
permissions in the release step if the v2 action requires them).
In @api/cards/productive-time.ts:
- Around line 23-40: The token is incorrectly initialized from
process.env.GITHUB_TOKEN!, which ignores multi-token setups; change the
initialization to call getGitHubToken(0) and use that variable everywhere in the
loop (replace the current token = process.env.GITHUB_TOKEN! with token =
getGitHubToken(0)), so getProductiveTimeSVGWithThemeName(username, theme,
Number(utcOffset), token) and the retry logic that calls
getGitHubToken(tokenIndex) work correctly.
In @api/cards/repos-per-language.ts:
- Around line 31-47: The code initializes token from process.env.GITHUB_TOKEN
which ignores numbered tokens and can be undefined; change the initialization to
use getGitHubToken(0) (or getGitHubToken(tokenIndex)) and keep tokenIndex in
sync so the retry loop rotates tokens correctly (update the token assignment
before the loop or set let tokenIndex = 0; let token =
getGitHubToken(tokenIndex); and continue to call getGitHubToken(tokenIndex) when
incrementing tokenIndex in the catch).
In @api/utils/github-token-updater.ts:
- Around line 1-11: Validate the incoming index in getGitHubToken (ensure it's a
non-negative integer) and determine which environment variable was actually used
before logging; replace the unconditional console.log(`Using ${tokenName}`) with
logic that sets a usedTokenName (either tokenName or 'GITHUB_TOKEN' when
fallback occurs) and log that, and throw a clear error if index is invalid or no
token is found; reference the getGitHubToken function, tokenName variable, the
fallback to process.env.GITHUB_TOKEN, and the console.log call to locate and
update the code.
In @package.json:
- Line 10: The npm script "lint" in package.json was widened from "src/**/*.ts"
to "**/*.ts", which now includes tests/; either revert the "lint" value back to
"src/**/*.ts" (targeting only source) or add "tests/" to .eslintignore so test
files are excluded—update the "lint" entry or .eslintignore accordingly and run
the linter to confirm only intended files are scanned.
- Around line 40-41: package.json was updated to major versions of TypeScript
ESLint packages ("@typescript-eslint/eslint-plugin" and
"@typescript-eslint/parser") which can be incompatible with current
ESLint/Prettier and project configs; verify compatibility by ensuring the ESLint
core and Prettier versions match required peer ranges, update ESLint and
Prettier configs (including rules like comma-dangle and formatting defaults) to
align with ESLint 8 and Prettier 3, then run npm run format-check and npm run
lint, fix any reported rule violations or formatting diffs, and pin or adjust
package versions if necessary to preserve CI formatting/linting behavior.
In @scripts/cleanup-vercel.sh:
- Around line 1-49: The script currently risks deleting the whole project by
calling vercel remove $PROJECT_NAME; instead, enable strict shell options (set
-euo pipefail), call vercel ls --json (omit the project name argument or ensure
correct target flags like --meta target=staging / --environment staging), parse
the JSON into deployment IDs/URLs using jq from the deployments variable, then
loop over each id and call vercel remove <deployment-id> --yes (not the project
name) to delete only individual staging/preview deployments; update prompts and
error handling accordingly.
In @src/app.ts:
- Around line 2-3: Add a single validated constant for GITHUB_TOKEN at module
init and fail fast if missing instead of using process.env.GITHUB_TOKEN! inline;
e.g., read once into a const (e.g., GITHUB_TOKEN) and if falsy log an
error/throw or process.exit(1), then replace all occurrences of
process.env.GITHUB_TOKEN! (including where Authorization headers are built and
where sendAnalytics or createProfileDetailsCard or other functions consume the
token) to use that validated const so you never emit "Authorization: bearer
undefined".
In @src/cards/profile-details-card.ts:
- Around line 63-79: getProfileDetailsData currently uses
profileDetails.contributionYears.slice(0, 1) which assumes years are
newest-first and can break when the array is empty; instead ensure you pick the
most recent year safely by checking profileDetails.contributionYears.length > 0
and selecting the max/newest year (e.g., sort descending or use
Array.prototype.reduce to find the largest year or use slice(-1) on a copy)
before calling getContributionByYear; apply the same guard/selection fix to the
other contribution-year loop (the later block that also iterates
contributionYears) so you never pass undefined to getContributionByYear and
always compute the intended latest-year contributions.
In @src/templates/profile-details-card.ts:
- Around line 52-54: The aggregation uses formatter/formatDate and then calls
new Date(formatDate), which is unreliable for "YYYY-MM" strings; replace the new
Date(formatDate) usage in the contributionsData loop (the code that computes
date.getTime() for grouping) with a deterministic parse such as using
d3.timeParse('%Y-%m')(formatDate) or by appending a day component (e.g., treat
month start by parsing formatDate + '-01') so the grouped timestamps are
consistent across environments.
In @src/utils/analytics.ts:
- Around line 7-37: sendAnalytics is currently including raw params.username in
the GA payload and sending engagement_time_msec and session_id as strings;
update sendAnalytics to compute clientId via getClientId(params.username) but
remove username from the event params before building payload (do not send PII),
and ensure engagement_time_msec and session_id are numeric values (e.g., 100 and
1) in the payload.events[0].params; modify the payload construction in
sendAnalytics (and avoid mutating the original params object by creating a
shallow copy without username) so GA receives pseudonymous client_id and numeric
fields.
🧹 Nitpick comments (12)
.github/workflows/manual-release.yml (1)
42-42: Consider addingfetch-depth: 0for tag operations.The
release-webjob specifiesfetch-depth: 0(line 28) but this job doesn't. Since you're creating releases with tags and generating release notes, a shallow clone may cause issues with tag creation or note generation that relies on commit history.Suggested fix
- uses: actions/checkout@v4 + with: + fetch-depth: 0README.md (1)
180-210: Prefernpx vercel(and clarify token expectations) to avoid global install + reduce friction.
Lines 206-210: instead ofnpm i -g vercel, consider documentingnpx vercel dev(ordevbox shell+ local install), and clarify what token type/scopes are expected forGITHUB_TOKENin local testing.api/cards/repos-per-language.ts (1)
24-28: Avoid type assertion + skip empty exclude entries.
Line 24: preferconst excludeArr: string[] = [];and filter out empty values soexclude=doesn’t add''to the exclude list.Possible tweak
- const excludeArr = <string[]>[]; - exclude.split(',').forEach(function (val) { - const translatedLanguage = translateLanguage(val); - excludeArr.push(translatedLanguage.toLowerCase()); - }); + const excludeArr: string[] = []; + exclude + .split(',') + .map((v) => v.trim()) + .filter(Boolean) + .forEach((val) => { + const translatedLanguage = translateLanguage(val); + excludeArr.push(translatedLanguage.toLowerCase()); + });tsconfig.test.json (1)
1-16: Looks good; consider excluding generated dirs if present (dist/build/coverage).
This config is fine for test/lint typechecking; adding common generated folders toexcludecan reduce noise if they exist in-repo.api/cards/profile-details.ts (1)
19-19: Consider explicit validation for GITHUB_TOKEN.The non-null assertion (
!) assumesGITHUB_TOKENis always present. While the outer try-catch would handle missing tokens, an explicit check with a clear error message would improve developer experience.♻️ Optional: Add explicit validation
- let token = process.env.GITHUB_TOKEN!; + const initialToken = process.env.GITHUB_TOKEN; + if (!initialToken) { + throw new Error('GITHUB_TOKEN environment variable is required'); + } + let token = initialToken;api/cards/stats.ts (1)
19-26: Consistent implementation with other card endpoints.The token management, analytics tracking, and cache control implementation follows the same pattern as
profile-details.ts, which ensures consistency across the codebase. The same optional refactor regarding explicitGITHUB_TOKENvalidation applies here as well.src/github-api/repos-per-language.ts (1)
33-61: Tighten types aroundfetcher/ pagination to avoid implicit-any drift.
Right nowvariables: any,cursor = null, andnodes = []make it easy for subtle response-shape regressions to slip through.Proposed refactor
-const fetcher = (token: string, variables: any) => { +type ReposPerLanguageVars = {login: string; endCursor: string | null}; +type ReposPerLanguageNode = {primaryLanguage: {name: string; color: string} | null}; + +const fetcher = (token: string, variables: ReposPerLanguageVars) => { // contain private repo need token permission return request( { Authorization: `bearer ${token}` }, { query: ` @@ variables } ); }; @@ export async function getRepoLanguages( username: string, exclude: Array<string>, token: string ): Promise<RepoLanguages> { let hasNextPage = true; - let cursor = null; + let cursor: string | null = null; const repoLanguages = new RepoLanguages(); - const nodes = []; + const nodes: ReposPerLanguageNode[] = [];Also applies to: 64-78
src/utils/analytics.ts (1)
39-48: Good: hard guard + timeout + error swallowing keeps analytics from breaking card generation.
One small hardening: ensure logs can’t accidentally include secrets (e.g., avoid dumping raw Axios error objects/config).src/cards/repos-per-language-card.ts (1)
6-13: Token propagation looks consistent; consider typinglangDataexplicitly.Proposed refactor
const getRepoLanguageData = async function (username: string, exclude: Array<string>, token: string) { const repoLanguages = await getRepoLanguages(username, exclude, token); - let langData = []; + let langData: {name: string; value: number; color: string}[] = [];Also applies to: 15-24, 31-33
src/app.ts (1)
126-133: Same token validation should apply tomain()too (local runs).
Otherwise local generation can silently degrade into unauthenticated/failed GitHub API calls.src/cards/most-commit-language-card.ts (1)
6-24: Token threading is consistent; consider typinglangDataexplicitly (avoid implicit any[]).Proposed refactor
const getCommitsLanguageData = async function ( username: string, exclude: Array<string>, token: string ): Promise<{name: string; value: number; color: string}[]> { const commitLanguages: CommitLanguages = await getCommitLanguage(username, exclude, token); - let langData = []; + let langData: {name: string; value: number; color: string}[] = [];Also applies to: 51-57
api/cards/most-commit-language.ts (1)
24-28: Avoid pushing empty/whitespace exclude entries; drop the cast-style init.
exclude=""currently yields['']and may create a no-op/odd exclude item.Proposed patch
- const excludeArr = <string[]>[]; - exclude.split(',').forEach(function (val) { - const translatedLanguage = translateLanguage(val); - excludeArr.push(translatedLanguage.toLowerCase()); - }); + const excludeArr: string[] = exclude + .split(',') + .map((val) => translateLanguage(val.trim()).toLowerCase()) + .filter(Boolean);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mapdist/licenses.txtis excluded by!**/dist/**dist/sourcemap-register.jsis excluded by!**/dist/**package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (36)
.eslintrc.json.github/workflows/manual-release.ymlREADME.mdapi/cards/most-commit-language.tsapi/cards/productive-time.tsapi/cards/profile-details.tsapi/cards/repos-per-language.tsapi/cards/stats.tsapi/utils/github-token-updater.tsdevbox.jsonjest.config.jspackage.jsonscripts/cleanup-vercel.shsrc/app.tssrc/cards/most-commit-language-card.tssrc/cards/productive-time-card.tssrc/cards/profile-details-card.tssrc/cards/repos-per-language-card.tssrc/cards/stats-card.tssrc/const/cache.tssrc/github-api/commits-per-language.tssrc/github-api/contributions-by-year.tssrc/github-api/productive-time.tssrc/github-api/profile-details.tssrc/github-api/repos-per-language.tssrc/templates/profile-details-card.tssrc/utils/analytics.tstests/cards/all-cards.test.tstests/github-api/commits-per-language.test.tstests/github-api/contributions-by-year.test.tstests/github-api/profile-details.test.tstests/github-api/repos-per-language.test.tstests/utils/analytics.test.tstsconfig.jsontsconfig.test.jsonvercel.json
🧰 Additional context used
🧬 Code graph analysis (17)
tests/github-api/profile-details.test.ts (1)
src/github-api/profile-details.ts (1)
getProfileDetails(92-127)
tests/github-api/repos-per-language.test.ts (1)
src/github-api/repos-per-language.ts (1)
getRepoLanguages(64-99)
tests/github-api/commits-per-language.test.ts (1)
src/github-api/commits-per-language.ts (1)
getCommitLanguage(64-97)
.github/workflows/manual-release.yml (1)
src/utils/file-writer.ts (1)
isInGithubAction(28-54)
tests/utils/analytics.test.ts (1)
src/utils/analytics.ts (1)
sendAnalytics(14-49)
tests/github-api/contributions-by-year.test.ts (1)
src/github-api/contributions-by-year.ts (1)
getContributionByYear(41-66)
src/cards/most-commit-language-card.ts (1)
src/github-api/commits-per-language.ts (2)
CommitLanguages(15-31)getCommitLanguage(64-97)
api/utils/github-token-updater.ts (1)
src/utils/file-writer.ts (1)
isInGithubAction(28-54)
api/cards/repos-per-language.ts (4)
src/utils/translator.ts (1)
translateLanguage(1-273)src/utils/analytics.ts (1)
sendAnalytics(14-49)src/const/cache.ts (1)
CONST_CACHE_CONTROL(1-1)api/utils/github-token-updater.ts (1)
getGitHubToken(1-11)
src/cards/profile-details-card.ts (3)
src/const/theme.ts (1)
ThemeMap(1-1)src/github-api/profile-details.ts (2)
ProfileDetails(3-25)getProfileDetails(92-127)src/github-api/contributions-by-year.ts (1)
getContributionByYear(41-66)
api/cards/productive-time.ts (3)
src/cards/productive-time-card.ts (1)
getProductiveTimeSVGWithThemeName(15-24)src/utils/analytics.ts (1)
sendAnalytics(14-49)src/const/cache.ts (1)
CONST_CACHE_CONTROL(1-1)
tests/cards/all-cards.test.ts (6)
src/utils/file-writer.ts (1)
writeSVG(14-18)src/cards/profile-details-card.ts (1)
createProfileDetailsCard(9-23)src/cards/repos-per-language-card.ts (1)
createReposPerLanguageCard(6-13)src/cards/most-commit-language-card.ts (1)
createCommitsPerLanguageCard(6-13)src/cards/stats-card.ts (1)
createStatsCard(9-16)src/cards/productive-time-card.ts (1)
createProductiveTimeCard(6-13)
api/cards/profile-details.ts (3)
src/cards/profile-details-card.ts (1)
getProfileDetailsSVGWithThemeName(24-33)src/utils/analytics.ts (1)
sendAnalytics(14-49)src/const/cache.ts (1)
CONST_CACHE_CONTROL(1-1)
api/cards/stats.ts (5)
src/cards/stats-card.ts (1)
getStatsSVGWithThemeName(18-22)src/utils/analytics.ts (1)
sendAnalytics(14-49)src/const/cache.ts (1)
CONST_CACHE_CONTROL(1-1)api/utils/github-token-updater.ts (1)
getGitHubToken(1-11)src/templates/stats-card.ts (1)
createStatsCard(5-61)
api/cards/most-commit-language.ts (4)
src/cards/most-commit-language-card.ts (1)
getCommitsLanguageSVGWithThemeName(15-24)src/utils/analytics.ts (1)
sendAnalytics(14-49)src/const/cache.ts (1)
CONST_CACHE_CONTROL(1-1)api/utils/github-token-updater.ts (1)
getGitHubToken(1-11)
src/cards/stats-card.ts (2)
src/github-api/profile-details.ts (1)
getProfileDetails(92-127)src/github-api/contributions-by-year.ts (1)
getContributionByYear(41-66)
src/app.ts (6)
src/cards/profile-details-card.ts (1)
createProfileDetailsCard(9-23)src/utils/analytics.ts (1)
sendAnalytics(14-49)src/cards/repos-per-language-card.ts (1)
createReposPerLanguageCard(6-13)src/cards/most-commit-language-card.ts (1)
createCommitsPerLanguageCard(6-13)src/cards/stats-card.ts (1)
createStatsCard(9-16)src/cards/productive-time-card.ts (1)
createProductiveTimeCard(6-13)
🪛 actionlint (1.7.10)
.github/workflows/manual-release.yml
64-64: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (40)
.github/workflows/manual-release.yml (1)
36-61: LGTM on job structure and build pipeline.The job setup follows good practices: appropriate permissions scoping, consistent git config with the other job, and graceful handling of "no changes" scenario with the
|| echofallback.tsconfig.json (1)
11-17: LGTM!Excluding
scripts/**/*.tsfrom the main compilation is appropriate for separating build-time utilities from the application codebase, especially with the introduction of a separate test configuration.devbox.json (1)
1-19: LGTM!The Devbox configuration properly defines a reproducible development environment with appropriate package versions and convenient shell scripts.
src/const/cache.ts (1)
1-1: LGTM!The cache control strategy is well-suited for profile card data: 4-hour client cache, 24-hour CDN cache, and 7-day stale-while-revalidate provide a good balance between freshness and performance. Centralizing this constant improves maintainability.
package.json (2)
13-13: LGTM!The
test:localscript provides a convenient way to test card generation locally, aligning with the Devbox development workflow.
54-55: Ensure TypeScript 5.4.5 build succeeds.Run
npm run buildandnpm testto confirm the upgrade doesn't introduce type errors or break existing functionality. The codebase uses strict mode and noImplicitAny, which should catch any compatibility issues.tests/github-api/commits-per-language.test.ts (1)
87-87: LGTM! Token parameter correctly added to test calls.The test calls have been properly updated to include the token parameter, aligning with the refactored function signature. Using a hardcoded 'token' string is appropriate for these unit tests since the HTTP requests are mocked.
Also applies to: 99-99, 104-104
.eslintrc.json (2)
7-14: LGTM! Improved formatting.The multi-line formatting of the
pluginsandextendsarrays improves readability.
19-19: tsconfig.test.json is properly configured for ESLint.Verification confirms the configuration meets all requirements:
- ✓ Extends the base
tsconfig.json- ✓ Includes both source and test files (
src/**/*,api/**/*,tests/**/*,scripts/**/*)- ✓ The
rootDiroverride to"."and intentional inclusion of test/api/scripts directories properly override the base config's exclusions, which is appropriate for ESLint's linting scopesrc/github-api/profile-details.ts (1)
92-95: LGTM! Explicit token passing improves testability.The function signature has been correctly updated to accept an explicit
tokenparameter, which is then passed to thefetcher. This refactoring removes the environment variable dependency and aligns with the broader token management improvements across the codebase.tests/github-api/repos-per-language.test.ts (1)
120-120: LGTM! Token parameter correctly added to test calls.The test calls have been properly updated to include the token parameter, aligning with the refactored function signature. The tests continue to properly exercise success, error, and language exclusion scenarios with appropriate mocked responses.
Also applies to: 132-132, 137-137
src/github-api/contributions-by-year.ts (1)
41-52: LGTM! Explicit token passing improves testability.The function signature has been correctly updated to accept an explicit
tokenparameter, which is then passed to thefetcher. This refactoring removes the environment variable dependency and maintains consistency with other API modules in the codebase.tests/github-api/contributions-by-year.test.ts (1)
37-37: LGTM! Test updates align with the new API signature.Both test cases correctly pass the token parameter to match the updated
getContributionByYearfunction signature.Also applies to: 47-47
tests/github-api/profile-details.test.ts (1)
73-73: LGTM! Test updates align with the new API signature.Both test cases correctly pass the token parameter to match the updated
getProfileDetailsfunction signature.Also applies to: 108-108
src/github-api/productive-time.ts (1)
72-92: LGTM! Token parameter correctly threaded through the data-fetching layer.The function signature update and token propagation to both GraphQL fetchers are consistent with the PR's token management refactor.
vercel.json (2)
6-11: LGTM! Formatting improvements enhance readability.The multi-line formatting for rewrites, headers, and redirects makes the configuration more maintainable without changing functionality.
Also applies to: 31-38, 55-58
12-17: Verify that resource limits are adequate for the card generation workload.The new function configuration sets memory to 128MB and maxDuration to 10 seconds for all API endpoints. Card generation involves multiple GraphQL queries, data processing, and SVG rendering for 30+ themes per request. Ensure these limits are sufficient to handle typical and peak workloads without timeout or memory issues.
Consider monitoring these metrics in production or testing with representative workloads to confirm the limits are appropriate.
tests/cards/all-cards.test.ts (3)
1-21: LGTM! Clean test setup with proper dependency mocking.The imports and mock setup follow Jest best practices by isolating the card creation functions from their dependencies.
53-55: Verify the productiveDate mock data type.The mock returns
productiveDate: [new Date().toISOString()](an array of strings), but theProfuctiveTimeclass definesproductiveDate: Date[](an array of Date objects). Ensure the card generation code handles whichever type is actually returned by the real implementation.Consider updating the mock to return Date objects for consistency:
🔧 Suggested fix
(getProductiveTime as jest.Mock).mockResolvedValue({ - productiveDate: [new Date().toISOString()] + productiveDate: [new Date()] });
58-103: LGTM! Comprehensive integration test coverage for all card types.The test suite verifies that each card creation function properly generates and writes SVG output. The consistent structure and appropriate assertions make the tests maintainable and effective.
tests/utils/analytics.test.ts (4)
1-13: LGTM!The test setup correctly uses
jest.resetModules()to ensure each test gets a fresh module instance with isolated environment variables. The pattern of saving and restoringprocess.envprevents test pollution.
15-36: LGTM!These tests correctly verify that analytics are skipped when environment guards fail. The use of dynamic
require()after setting environment variables is the correct pattern for testing module-level code that reads fromprocess.env.
38-66: LGTM!The test comprehensively verifies the GA4 payload structure, including the measurement ID in the URL, event structure, and parameter propagation. The use of flexible matchers (
expect.any,expect.objectContaining) appropriately handles computed values like the hashedclient_id.
68-84: LGTM!The error handling test correctly verifies that analytics errors are caught and logged without breaking the main flow. Proper mock restoration ensures no test pollution.
src/github-api/commits-per-language.ts (2)
64-68: LGTM!The function signature correctly adds the
tokenparameter to support the new token management strategy. This breaking change aligns with the PR's refactoring objective and enables caller-controlled authentication.
71-73: LGTM!The implementation correctly passes the provided token to the fetcher, removing the implicit dependency on environment variables. This change supports the PR's goal of explicit token management.
jest.config.js (2)
8-10: LGTM!The updated ts-jest configuration uses the modern array format, enabling test-specific TypeScript compiler options via
tsconfig.test.json. This is the recommended approach for configuring ts-jest with custom settings.
16-16: LGTM!Excluding the
scripts/**directory from coverage collection is appropriate, as utility scripts typically don't require the same test coverage as application code.api/cards/profile-details.ts (4)
2-5: LGTM!The updated imports correctly reflect the new token management utility (
getGitHubToken), analytics tracking (sendAnalytics), and centralized cache control constant (CONST_CACHE_CONTROL).
23-24: LGTM!The token is correctly threaded through to the card generation function, and analytics are properly awaited to ensure the event is sent before the Lambda function freezes.
26-26: LGTM!Using the centralized
CONST_CACHE_CONTROLconstant ensures consistent cache behavior across all card endpoints and makes cache policy updates easier.
32-34: LGTM!The token rotation logic correctly increments the index and retrieves the next token using
getGitHubToken(). This enables multi-token setups for handling rate limits.api/cards/stats.ts (2)
2-5: LGTM!The updated imports mirror the pattern in
profile-details.ts, providing consistent token management, analytics, and cache control across card endpoints.
32-34: LGTM!The token rotation logic matches the pattern used in other card endpoints, enabling rate limit handling through multi-token rotation.
src/cards/productive-time-card.ts (2)
6-13: Token threading here looks consistent with the rest of the PR.
15-24: Good: token forwarded through the themed SVG entrypoint.src/cards/stats-card.ts (1)
9-22: Token propagation intogetProfileDetails/getContributionByYearis consistent across Vercel/non-Vercel paths.Also applies to: 33-55
api/cards/most-commit-language.ts (1)
10-23: Input validation looks good (string checks for query params).
This keeps the handler from accidentally accepting array/querystring edge-cases.src/cards/profile-details-card.ts (2)
9-33: Token threading in public APIs is consistent and clean.
Signatures and downstream calls line up with the new token-aware GitHub API functions.
45-61: Formatting-only change here; logic unchanged.
No concerns.
|
Updated title and added docstrings to improve coverage. |
1 participant
This pull request introduces several improvements and refactors across the codebase, focusing on enhanced GitHub token management, improved analytics, better cache control, and streamlined development workflows. The most significant changes include replacing the GitHub token rotation logic, standardizing cache headers and analytics tracking for API endpoints, updating the development environment setup, and modernizing dependencies and linting configurations.
API Endpoint Improvements:
changToNextGitHubTokenfunction with a newgetGitHubTokenutility, improving how API endpoints manage and rotate GitHub tokens for rate-limiting and multi-token setups. All relevant API card endpoints now use this new approach. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]sendAnalytics) and standardized cache headers (CONST_CACHE_CONTROL) to all card API endpoints for better observability and cache consistency. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]Development Workflow and Tooling:
devbox.jsonconfiguration for reproducible local development environments, ensuring consistent Node.js and Python versions and pre-commit hooks.README.mdto document the new Devbox-based development workflow and added instructions for local testing and running the Vercel API locally.test:localscript for local card generation and a shell script (scripts/cleanup-vercel.sh) for cleaning up Vercel preview deployments. [1] [2]Dependency and Linting Updates:
childprocess,moment) and updated TypeScript and related dev dependencies to latest versions. [1] [2]CI/CD Enhancements:
release-actionjob to the GitHub Actions workflow for building, packaging, committing, and releasing the distribution on demand.These changes collectively improve maintainability, reliability, and developer experience for the project.
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.