-
Notifications
You must be signed in to change notification settings - Fork 439
feat(web): update GitHub stats and stargazers fetching logic #2113
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
Conversation
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds a new backend Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Web Client
participant Hypr as HyprNote stub (api.hyprnote.com)
participant API as Backend API (/stargazers)
participant Cache as In-memory Cache
participant GitHub as GitHub API
Note over Browser,Hypr: Frontend stargazers fetch (two-source)
Browser->>Hypr: GET /stargazers (HyprNote stub)
alt HyprNote returns stargazers
Hypr-->>Browser: 200 { stargazers }
else HyprNote fails
Browser->>GitHub: GET repo stargazers (fallback)
GitHub-->>Browser: paginated stargazers -> map to {username, avatar}
end
Note over Browser,API: Alternative path using backend endpoint
Browser->>API: GET /stargazers
API->>Cache: check cache for fresh entry
alt Cache hit (fresh)
Cache-->>API: stargazers
API-->>Browser: 200 { stargazers }
else Cache miss or stale
API->>GitHub: GET repo + paginated stargazers
GitHub-->>API: pages of stargazers
API->>API: aggregate, reverse, store in Cache (TTL)
API-->>Browser: 200 { stargazers }
else GitHub fetch failure
alt Cache has stale data
Cache-->>API: stale stargazers
API-->>Browser: 200 { stargazers (stale) }
else No cache
API-->>Browser: 5xx error
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/src/queries.ts (1)
26-36: Fallback fetches oldest stargazers vs. API's most recent.The API endpoint fetches the most recent 512 stargazers (via pagination from the last pages), while this fallback fetches the first 100 (oldest). This inconsistency may cause different avatars to display when falling back.
If consistency matters, consider fetching from later pages or documenting this as expected fallback behavior.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api/src/routes.ts(3 hunks)apps/web/src/queries.ts(2 hunks)apps/web/src/routes/_view/opensource.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/routes/_view/opensource.tsxapps/api/src/routes.tsapps/web/src/queries.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/api/src/routes.tsapps/web/src/queries.ts
🧬 Code graph analysis (2)
apps/web/src/routes/_view/opensource.tsx (1)
apps/web/src/queries.ts (1)
useGitHubStargazers(38-54)
apps/api/src/routes.ts (1)
apps/web/src/queries.ts (1)
GITHUB_ORG_REPO(56-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: ci
- GitHub Check: fmt
🔇 Additional comments (8)
apps/web/src/routes/_view/opensource.tsx (1)
126-126: LGTM!The updated hook call aligns with the new
useGitHubStargazers()signature that no longer accepts a count parameter. The grid rendering logic at lines 101-102 uses modulo indexing (% stargazers.length), so it handles any array size gracefully.apps/api/src/routes.ts (4)
12-12: LGTM!The new
PUBLICtag appropriately categorizes the unauthenticated/stargazersendpoint.
34-42: LGTM!Using
z.looseObject()is the correct Zod 4 approach for allowing additional properties on OpenAI-compatible requests. This aligns with the library's updated API.
362-367: Good resilience pattern with stale cache fallback.Serving stale cached data on fetch failure ensures the endpoint remains functional during GitHub API outages. This is a solid approach.
17-23: In-memory cache works effectively for this deployment model.The module-level
stargazerCachepersists across requests in the Bun runtime with a traditional long-running server process. This implementation is appropriate for the current setup and effectively reduces redundant GitHub API calls. No distributed cache is needed unless the deployment model changes to a serverless/edge function architecture.Likely an incorrect or invalid review comment.
apps/web/src/queries.ts (3)
4-5: LGTM!Updated baseline values for stars and forks are reasonable snapshots for fallback display.
38-53: LGTM!The two-tier fetch strategy with graceful fallback is well-implemented. The
staleTimeof 1 hour aligns with the backend cache TTL, and error handling properly catches both network failures and non-ok responses.
56-58: LGTM!Exporting these constants provides a clean public API for consumers needing repository metadata.
550150e to
7021297
Compare
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.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/api/src/routes.ts (1)
392-433: GitHub API rate limiting risk without authentication.As noted in the previous review, unauthenticated GitHub API requests are rate-limited to 60/hour, which could be quickly exhausted in serverless environments with ephemeral caches.
🧹 Nitpick comments (4)
apps/api/src/routes.ts (3)
414-414: Document the hardcoded stargazer count.The
count = 512is hardcoded without explanation. Consider making this a named constant with a comment explaining the choice, or making it configurable via query parameter or environment variable.Apply this diff to extract the constant:
+const MAX_STARGAZERS_COUNT = 512; // Fetch last 512 stargazers to populate the grid + routes.get( "/stargazers", describeRoute({And update line 414:
- const count = 512; + const count = MAX_STARGAZERS_COUNT;
438-447: Add error logging for failed page fetches.Individual page fetch failures are silently ignored (line 439). While graceful degradation is good, these failures should be logged for observability.
Apply this diff:
for (const response of responses) { - if (!response.ok) continue; + if (!response.ok) { + Sentry.captureMessage( + `Failed to fetch stargazers page: ${response.status}`, + { level: "warning" }, + ); + continue; + } const data = await response.json();
457-462: Add error logging in catch block.The catch block silently suppresses all errors. Log the error before falling back to cached data or returning an error response.
Apply this diff:
- } catch { + } catch (error) { + Sentry.captureException(error, { + tags: { endpoint: "stargazers" }, + }); if (stargazerCache) {apps/web/src/queries.ts (1)
26-36: Fallback fetches fewer stargazers than primary source.The
fetchStargazersFromGitHubfallback fetches only 100 stargazers, while the HyprNote API endpoint returns up to 512. This discrepancy means users may see different amounts of data depending on which source is used.Consider whether the fallback should match the API's count or if the difference is acceptable given this is a degraded fallback scenario.
If matching is desired, apply this diff to fetch more pages:
async function fetchStargazersFromGitHub(): Promise<Stargazer[]> { - const response = await fetch( - `https://api.github.com/repos/${ORG_REPO}/stargazers?per_page=100`, - ); - if (!response.ok) return []; - const data = await response.json(); - return data.map((user: { login: string; avatar_url: string }) => ({ - username: user.login, - avatar: user.avatar_url, - })); + try { + const perPage = 100; + const pages = 6; // Fetch 600 stargazers (more than API's 512) + const promises = []; + for (let i = 1; i <= pages; i++) { + promises.push( + fetch( + `https://api.github.com/repos/${ORG_REPO}/stargazers?per_page=${perPage}&page=${i}`, + ), + ); + } + const responses = await Promise.all(promises); + const allData = await Promise.all( + responses + .filter((r) => r.ok) + .map((r) => r.json()), + ); + return allData + .flat() + .map((user: { login: string; avatar_url: string }) => ({ + username: user.login, + avatar: user.avatar_url, + })); + } catch { + return []; + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api/src/routes.ts(3 hunks)apps/web/src/queries.ts(2 hunks)apps/web/src/routes/_view/opensource.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/routes/_view/opensource.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/api/src/routes.tsapps/web/src/queries.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/api/src/routes.tsapps/web/src/queries.ts
🧬 Code graph analysis (1)
apps/api/src/routes.ts (1)
apps/web/src/queries.ts (1)
GITHUB_ORG_REPO(56-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
🔇 Additional comments (6)
apps/api/src/routes.ts (4)
23-23: LGTM!The PUBLIC tag addition is appropriate for the new stargazers endpoint.
26-34: Note: Module-level cache is ephemeral in serverless environments.The module-level
stargazerCacheworks correctly but will be instance-specific in serverless deployments. Each cold start creates a new cache, so the effective cache behavior depends on instance lifetime and request routing.This is acceptable for the current use case, but be aware that:
- Cache warming doesn't persist across instances
- Different users may see different cache states
- The 1-hour TTL helps mitigate stale data concerns
350-357: LGTM!The schema correctly models the stargazers response structure.
45-53: Zod version confirmed —z.looseObject()is the correct API.The project uses Zod 4.1.13, which fully supports
z.looseObject()for accepting unknown properties on object schemas. The migration from.passthrough()is appropriate for this version.apps/web/src/queries.ts (2)
4-5: LGTM!The updated last-seen values provide reasonable fallbacks for the GitHub stats.
38-54: Breaking change: Query key and function signature updated.The function signature changed from
useGitHubStargazers(count?: number)touseGitHubStargazers(), and the query key changed from["github-stargazers", count]to["github-stargazers"]. This will invalidate existing cached queries.The two-source fetch pattern (HyprNote API → GitHub fallback → empty array) provides robust error handling with graceful degradation.
Note: Ensure all call sites have been updated to match the new signature (the PR summary indicates
apps/web/src/routes/_view/opensource.tsxwas updated).
7021297 to
ad802d5
Compare
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/routes/_view/opensource.tsx (1)
100-117: Random delay computed during render will cause inconsistent animations on re-renders.
Math.random() * 3is called inline during render (line 103), meaning each re-render generates new delay values, potentially causing visual jitter. Consider memoizing the delays.+import { useMemo } from "react"; + function StargazersGrid({ stargazers }: { stargazers: Stargazer[] }) { const rows = 10; const cols = 20; + + const delays = useMemo( + () => Array.from({ length: rows * cols }, () => Math.random() * 3), + [rows, cols] + ); return ( <div className="absolute inset-0 overflow-hidden pointer-events-none"> <div className="absolute inset-0 flex flex-col justify-center gap-1 opacity-40 px-4"> {Array.from({ length: rows }).map((_, rowIndex) => ( <div key={rowIndex} className="flex gap-1 justify-center"> {Array.from({ length: cols }).map((_, colIndex) => { const index = (rowIndex * cols + colIndex) % stargazers.length; const stargazer = stargazers[index]; - const delay = Math.random() * 3; + const delay = delays[rowIndex * cols + colIndex]; return (
♻️ Duplicate comments (1)
apps/api/src/routes.ts (1)
392-433: Acknowledged: GitHub API rate limiting concern.The previous review correctly identified the rate limiting risk with unauthenticated GitHub API requests (60/hour limit). Adding optional
GITHUB_TOKENauthentication as suggested would significantly improve reliability.
🧹 Nitpick comments (1)
apps/api/src/routes.ts (1)
26-34: Consider importingGITHUB_ORG_REPOfrom a shared location.The constant
GITHUB_ORG_REPOis duplicated here and inapps/web/src/queries.ts. If this value ever changes, it could lead to inconsistencies. Consider extracting to a shared constants file.The in-memory cache will reset on serverless cold starts, but the fallback to stale data on fetch failure (lines 458-460) provides reasonable resilience.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/api/src/routes.ts(3 hunks)apps/web/src/components/download-button.tsx(1 hunks)apps/web/src/queries.ts(2 hunks)apps/web/src/routes/_view/opensource.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/components/download-button.tsxapps/api/src/routes.tsapps/web/src/routes/_view/opensource.tsxapps/web/src/queries.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/api/src/routes.tsapps/web/src/queries.ts
🧠 Learnings (2)
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: Applies to **/*.{ts,tsx} : If there are many classNames with conditional logic, use `cn` (import from `hypr/utils`), similar to `clsx`. Always pass an array and split by logical grouping.
Applied to files:
apps/web/src/routes/_view/opensource.tsx
📚 Learning: 2025-11-24T16:32:23.055Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:23.055Z
Learning: Applies to **/*.{ts,tsx} : If there are many classNames with conditional logic, use `cn` (import from `hypr/utils`). It is similar to `clsx`. Always pass an array and split by logical grouping.
Applied to files:
apps/web/src/routes/_view/opensource.tsx
🧬 Code graph analysis (2)
apps/api/src/routes.ts (1)
apps/web/src/queries.ts (1)
GITHUB_ORG_REPO(98-98)
apps/web/src/routes/_view/opensource.tsx (1)
apps/web/src/queries.ts (1)
useGitHubStargazers(80-96)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: Redirect rules - hyprnote-storybook
- GitHub Check: Header rules - hyprnote-storybook
- GitHub Check: ci
- GitHub Check: fmt
- GitHub Check: Pages changed - hyprnote-storybook
🔇 Additional comments (8)
apps/web/src/components/download-button.tsx (1)
45-50: LGTM!The styling adjustment removing text sizing classes aligns with the broader UI consistency changes in this PR. The
cnutility is correctly used with an array pattern per coding guidelines.apps/api/src/routes.ts (1)
45-53: LGTM!Using
z.looseObjectis appropriate for an OpenAI-compatible endpoint that may receive additional properties from various clients. This aligns with Zod 4's recommended pattern for loose validation.apps/web/src/queries.ts (3)
4-5: LGTM!Updated fallback values for star/fork counts.
26-78: LGTM!The fallback GitHub fetching logic is well-structured:
- Correctly parses the Link header for pagination
- Efficiently fetches only the last 2 pages needed
- Properly limits results to 200 (matching the 10×20 grid)
- Good error handling returning empty arrays on failure
80-96: LGTM!The two-tier fetching strategy (HyprNote API first, GitHub fallback) provides good reliability. The
staleTimeof 1 hour matches the API's cache TTL, ensuring consistency. Error handling gracefully degrades to empty arrays.apps/web/src/routes/_view/opensource.tsx (3)
73-89: LGTM!The increased avatar size (
size-14) provides better visual prominence for stargazers.
125-127: LGTM!Hook usage correctly updated to match the new parameterless signature.
149-153: LGTM!Styling adjustment aligns with the broader UI consistency changes. The
cnutility is correctly used with an array pattern per coding guidelines.
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/api/src/routes.ts (1)
437-446: Add logging for failed page fetches.Individual page fetch failures are silently skipped (line 438), which reduces observability. If some pages fail while others succeed, the endpoint returns partial data without indication. This makes debugging GitHub API issues difficult.
Add console logging for failed fetches:
for (const response of responses) { - if (!response.ok) continue; + if (!response.ok) { + console.warn(`Failed to fetch stargazers page: ${response.status} ${response.url}`); + continue; + } const data = await response.json();Note: This issue was previously flagged and remains unaddressed.
🧹 Nitpick comments (2)
apps/api/src/routes.ts (2)
26-34: Cache implementation is acceptable for single-threaded runtime.The module-level mutable cache lacks synchronization, which could theoretically lead to race conditions during concurrent updates. However, this is acceptable because:
- JavaScript runtimes are single-threaded (async operations don't cause concurrent mutations)
- The worst case is redundant GitHub fetches or serving briefly stale data
- The data is non-critical
If scaling to multi-process/cluster deployments, consider using Redis or atomic cache updates.
418-432: Consider extracting magic numbers to named constants.The pagination logic is correct, but the hardcoded values
512(desired stargazers) and100(per page) could be extracted to named constants for better maintainability.+const DESIRED_STARGAZERS_COUNT = 512; +const GITHUB_STARGAZERS_PER_PAGE = 100; + try { // ... repo fetch ... - const count = 512; - const perPage = 100; + const count = DESIRED_STARGAZERS_COUNT; + const perPage = GITHUB_STARGAZERS_PER_PAGE; const numPages = Math.ceil(Math.min(count, totalStars) / perPage);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/src/routes.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/api/src/routes.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/api/src/routes.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: fmt
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: ci
- GitHub Check: Pages changed - hyprnote
🔇 Additional comments (4)
apps/api/src/routes.ts (4)
23-23: LGTM!The addition of the PUBLIC tag is appropriate for categorizing the new public stargazers endpoint.
45-53: LGTM! Correct Zod 4 pattern for allowing extra fields.The change to
z.looseObjectaligns with Zod 4 best practices for object schemas that accept unknown keys. This is functionally equivalent to the previous.passthrough()pattern and correctly preserves extra fields captured by...restBodyat line 144.
350-357: LGTM!The schema correctly models the stargazers response structure and aligns with the data transformation logic.
392-399: LGTM! GitHub token support properly implemented.The optional token authentication correctly addresses the rate limiting concern raised in previous reviews. The implementation properly falls back to unauthenticated requests when no token is available.
No description provided.