feat: add user count API endpoint and refactor stats fetching logic:#241
Conversation
- Introduced a new API endpoint to count users in the database. - Updated the stats fetching logic to use the new endpoint, improving error handling with a fallback mechanism. - Enhanced the overall structure of the getStats function for better readability and maintainability.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
WalkthroughA terminal server endpoint now exposes the database user count. The web stats action retrieves users, npm downloads, and GitHub stars through cached HTTP requests with fallback values. ChangesStats API migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant getStats
participant TerminalServer
participant Prisma
participant NpmAPI
participant GitHubAPI
getStats->>TerminalServer: Fetch user count
TerminalServer->>Prisma: Count users
Prisma-->>TerminalServer: Return count
TerminalServer-->>getStats: Return user count JSON
getStats->>NpmAPI: Fetch downloads
NpmAPI-->>getStats: Return downloads
getStats->>GitHubAPI: Fetch stargazers
GitHubAPI-->>getStats: Return stars
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/supercode-cli/server/src/index.ts`:
- Around line 125-133: Update the user-count route around the app.get handler to
use the existing Bun.serve() routing structure instead of Express, preserving
its Prisma user count response and 500-error behavior. Integrate the route into
the server’s existing routes while retaining WebSocket support, or use the
repository’s established exemption mechanism if migration is not possible.
In `@apps/web/modules/stats/actions/index.ts`:
- Around line 13-15: Update getStats to resolve the terminal-server URL using
the same resolver and fallback chain as the analytics route, rather than
defaulting directly to http://localhost:3004. Reuse the existing shared
URL-resolution logic so the fallback aligns with the terminal server’s
PORT=10000 default and requests the correct users endpoint.
- Around line 3-10: Update safeFetchJson to return fallback when the fetch
response is not OK before parsing JSON. In getStats, validate each fetched
payload with the existing or appropriate Zod schemas before performing fallback
arithmetic, so invalid fields such as string counts use the documented fallback
instead of being coerced.
- Around line 3-5: Update the stats page’s caching configuration so
safeFetchJson’s revalidate: 3600 can establish hourly caching: remove the
force-dynamic export from the stats page when static generation is supported, or
move the cached getStats/safeFetchJson fetch behind a route or Server Component
boundary that is not forced dynamic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f9fdd136-9587-4c37-86bf-877321ff8189
📒 Files selected for processing (2)
apps/supercode-cli/server/src/index.tsapps/web/modules/stats/actions/index.ts
| app.get("/api/data/users/count", async (_req, res) => { | ||
| try { | ||
| const count = await prisma.user.count() | ||
| res.json({ count }) | ||
| } catch (error) { | ||
| console.error("[users/count] Error:", error) | ||
| res.status(500).json({ error: "Failed to fetch user count" }) | ||
| } | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)apps/supercode-cli/server/src/index\.ts$|(^|/)apps/supercode-cli/server/src/server\.ts$|package\.json$|bun\.lockb$' || true
echo "== index outline =="
if [ -f apps/supercode-cli/server/src/index.ts ]; then
wc -l apps/supercode-cli/server/src/index.ts
ast-grep outline apps/supercode-cli/server/src/index.ts || true
echo "== relevant index =="
sed -n '1,220p' apps/supercode-cli/server/src/index.ts
fi
echo "== route/framework references =="
rg -n "app\.get|Express|express|Bun\.serve|WebSocket|prisma\.user\.count|users/count" apps/supercode-cli/server/src/index.ts apps/supercode-cli/server 2>/dev/null | head -200 || trueRepository: yashdev9274/supercli
Length of output: 13398
Use Bun.serve() for the new route.
This handler is added via Express’s app.get, but apps/supercode-cli/server/src/index.ts matches the repository rule requiring Bun.serve() with routes and WebSocket support instead of Express. Migrate this route with the server implementation or provide an explicit exemption.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/supercode-cli/server/src/index.ts` around lines 125 - 133, Update the
user-count route around the app.get handler to use the existing Bun.serve()
routing structure instead of Express, preserving its Prisma user count response
and 500-error behavior. Integrate the route into the server’s existing routes
while retaining WebSocket support, or use the repository’s established exemption
mechanism if migration is not possible.
Source: Coding guidelines
| async function safeFetchJson<T>(url: string, fallback: T): Promise<T> { | ||
| try { | ||
| const res = await fetch(url, { next: { revalidate: 3600 } }) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate stats page and actions =="
fd -a 'page\.tsx$|index\.ts$' apps/web/modules/apps/web 2>/dev/null | sed 's#^\./##' | rg 'apps/web/app/\(pages\)/stats/page\.tsx|apps/web/modules/stats/actions/index\.ts' || true
echo
echo "== stats page outline/content =="
if [ -f 'apps/web/app/(pages)/stats/page.tsx' ]; then
wc -l 'apps/web/app/(pages)/stats/page.tsx'
cat -n 'apps/web/app/(pages)/stats/page.tsx' | sed -n '1,220p'
fi
echo
echo "== stats actions content =="
if [ -f 'apps/web/modules/stats/actions/index.ts' ]; then
wc -l 'apps/web/modules/stats/actions/index.ts'
cat -n 'apps/web/modules/stats/actions/index.ts' | sed -n '1,180p'
fiRepository: yashdev9274/supercli
Length of output: 1840
🌐 Web query:
Next.js App Router force-dynamic fetch next revalidate revalidate ignored route segment config
💡 Result:
In the Next.js App Router, the interaction between individual fetch revalidation settings and route segment configuration—particularly force-dynamic—has evolved through various releases. Here is how they function together. Understanding Segment vs. Fetch Settings The route segment config (e.g., export const revalidate or export const dynamic) acts as a default or constraint for the entire route, while the fetch options are granular, per-request settings [1][2]. Impact of force-dynamic When a route is marked with export const dynamic = 'force-dynamic', Next.js forces the route to be rendered dynamically on every request [1]. Historically, this configuration caused all fetch requests within that segment to opt out of caching [3][4]. Precedence and Recent Changes Recent updates (notably PR #72357) have refined this behavior to allow more explicit control: 1. Fetch Precedence: If you explicitly set a positive revalidate value on a fetch request (e.g., next: { revalidate: 60 }) inside a force-dynamic route, the revalidate setting is now intended to take precedence, allowing that specific request to be cached even if the route is otherwise dynamic [5]. 2. Opting into Cache: For this to work reliably when force-dynamic is present, you should generally ensure you are not conflicting with other cache-disabling settings and may need to explicitly use cache: 'force-cache' in your fetch call if the default behavior is being overridden by the segment config [5]. Common Pitfalls - Segment as Ceiling: When both segment-level revalidate and fetch-level revalidate are set, the effective revalidation time is the lower of the two values [1][6]. If the segment config is set to a specific number (e.g., revalidate = 60), a fetch with revalidate = 3600 will still revalidate every 60 seconds [1]. - Conflicting Options: Setting both { revalidate: 3600, cache: 'no-store' } in a single fetch call will cause the options to be ignored, and Next.js will typically log a warning in development mode [7]. - Development vs. Production: Behavior can sometimes differ in next dev due to how the development server handles cache headers and HMR [8]. Always verify caching behavior using production builds (next build && next start) [2]. If you find your fetch revalidation is being ignored, check for conflicting settings (like cache: 'no-store') and ensure your Next.js version is up to date, as issues regarding force-dynamic overriding fetch cache have been subject to frequent framework patches [9][5][3].
Citations:
- 1: https://juanchi.dev/en/blog/nextjs-app-router-caching-revalidate-dynamic-no-store
- 2: https://dev.to/mudassirworks/nextjs-app-router-caching-what-actually-gets-cached-and-what-doesnt-6ej
- 3: vercel/next.js@f602b29
- 4: fix: fetch() behavior when "dynamic" is "force-dynamic vercel/next.js#58484
- 5: vercel/next.js@19f93c8
- 6: https://nextjs.im/docs/15/app/api-reference/file-conventions/route-segment-config/
- 7: https://nextjs.org/docs/app/api-reference/functions/fetch
- 8: How does mixed of `force-dynamic` and `revalidate:60` in fetch API works? vercel/next.js#53631
- 9: basic cache fetch revalidation not working in 13.2.2 client component vercel/next.js#47354
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package versions =="
if [ -f package.json ]; then
node - <<'JS'
const fs = require('fs');
const p = JSON.parse(fs.readFileSync('package.json','utf8'));
console.log('root next:', p.dependencies?.next || p.devDependencies?.next);
console.log('root react:', p.dependencies?.react || p.devDependencies?.react);
for (const f of ['apps/web/package.json', 'apps/web/package-lock.json', 'package-lock.json']) {
if (fs.existsSync(f)) {
console.log('\nfile:', f);
const pkg = JSON.parse(fs.readFileSync(f,'utf8'));
if (pkg.dependencies?.next || pkg.devDependencies?.next) {
console.log('next:', pkg.dependencies?.next || pkg.devDependencies?.next);
}
}
}
JS
fi
echo
echo "== next config =="
fd -a 'next\.config\.(js|mjs|ts)$' apps/web . | sed 's#^\./##' | while read -r f; do
echo "--- $f"
wc -l "$f"
cat -n "$f" | sed -n '1,220p'
doneRepository: yashdev9274/supercli
Length of output: 2817
🌐 Web query:
apps/web next.config.ts is server app page config dynamic force-dynamic Next.js 15 route segment config
💡 Result:
In Next.js 15, the dynamic configuration is a Route Segment Config option, not a setting defined in next.config.ts [1][2][3]. To force a page, layout, or route handler to be dynamically rendered at request time, you must export the dynamic constant directly from the route segment file (e.g., page.tsx, layout.tsx, or route.ts) [1][4]. Use the following syntax in your page or layout file: export const dynamic = 'force-dynamic'; When this is set, the route will be rendered for each user at request time [1][5]. This is functionally equivalent to setting every fetch request within that segment to { cache: 'no-store', next: { revalidate: 0 } } and setting the fetchCache option to 'force-no-store' [1]. Key points regarding this configuration: - Next.config.ts: This file is for global project-level configuration (e.g., experimental flags, redirects, headers) and does not control the dynamic rendering behavior of individual routes [2][3]. - Purpose: 'force-dynamic' is intended for routes that require request-time data, such as headers, cookies, or frequently changing server-side data [5]. - Alternatives: Using export const revalidate = 0 in your route segment provides an equivalent outcome, forcing the route to be dynamic [5][6]. - Next.js 15 behavior: Note that in Next.js 15, GET Route Handlers are not cached by default [2]. If you only need to ensure data freshness for specific fetches, you can use the { cache: 'no-store' } option on those individual fetch calls rather than forcing the entire route to be dynamic [1][6].
Citations:
- 1: https://nextjs.org/docs/15/app/api-reference/file-conventions/route-segment-config
- 2: https://nextjs.org/blog/next-15
- 3: https://nextjs.org/docs/app/api-reference/config/next-config-js
- 4: https://nextjs.org/docs/app/api-reference/file-conventions/route-segment-config
- 5: https://dev.to/mahdi_benrhouma_fe1c6005/fix-dynamic-server-usage-error-in-nextjs-app-router-1jna
- 6: https://tutorial.techaltum.com/nextjs-data-fetching.html
Avoid forcing hourly stats revalidation into no-store.
apps/web/app/(pages)/stats/page.tsx exports dynamic = "force-dynamic", and its fetch-like calls all originate from that route (including getStats()), so the explicit next: { revalidate: 3600 } is unlikely to establish the intended hourly cache. Remove force-dynamic if the page can be statically generated, or move the cached fetch behind a route/Server Component boundary that is not forced dynamic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/modules/stats/actions/index.ts` around lines 3 - 5, Update the stats
page’s caching configuration so safeFetchJson’s revalidate: 3600 can establish
hourly caching: remove the force-dynamic export from the stats page when static
generation is supported, or move the cached getStats/safeFetchJson fetch behind
a route or Server Component boundary that is not forced dynamic.
Source: MCP tools
| async function safeFetchJson<T>(url: string, fallback: T): Promise<T> { | ||
| try { | ||
| const res = await fetch(url, { next: { revalidate: 3600 } }) | ||
| return await res.json() | ||
| } catch { | ||
| return fallback | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate target file and related files"
git ls-files | rg '(^|/)apps/web/modules/stats/actions/index\.ts$|package.json$|zod|tsconfig'
echo
echo "Target file outline"
ast-grep outline apps/web/modules/stats/actions/index.ts 2>/dev/null || true
echo
echo "Target file content"
cat -n apps/web/modules/stats/actions/index.ts
echo
echo "Search stats/json5/500/fetch usages"
rg -n "safeFetchJson|getStats|force-dynamic|fetch\(|json5|JSON\.parse|500|revalidate|localhost:3004" apps/web/modules/stats apps/web/features apps/web -g '*.ts' -g '*.tsx' | head -200Repository: yashdev9274/supercli
Length of output: 15299
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "apps/web/package.json relevant deps"
node - <<'JS'
const fs = require('fs')
const p = JSON.parse(fs.readFileSync('apps/web/package.json', 'utf8'))
const deps = {...(p.dependencies||{}), ...(p.devDependencies||{})}
for (const [k,v] of Object.entries(deps)) {
if (/^(next|zod|json5)$/.test(k)) console.log(`${k}: ${v}`)
}
JS
echo
echo "Behavioral probe: fetch response handling and numeric coercion"
node - <<'JS'
class Response {
constructor(status, json, err) {
this.status = status
this._json = json
this._err = err
this.ok = status >= 200 && status < 300
}
async json() {
if (this._err) throw this._err
return this._json
}
}
async function currentSafeFetchJson(url, fallback) {
// Minimal JS equivalent of the current helper: no status check
const res = new Response(500, { error: "Internal Server Error" }, undefined)
try {
return await res.json()
} catch {
return fallback
}
}
for (const payload of [
{ status: 500, json: { downloads: "not a number" }, fallback: { downloads: 0 } },
{ status: 200, json: { downloads: NaN }, fallback: { downloads: 0 } },
{ status: 200, json: { downloads: null }, fallback: { downloads: 0 } },
{ status: "network error thrown", json: null, fallback: { downloads: 0 } },
]) {
let actualPayload
try {
actualPayload = payload.json
} catch (e) {
actualPayload = `THREW:${e.message}`
}
const fallbackReturn = typeof actualPayload === "object" && actualPayload !== null ? actualPayload : payload.fallback
const userStats = fallbackReturn
const computed = (userStats.downloads ? 1 : 0) + 6000
const arithmeticSafe = (userStats.downloads ?? 0) + 6000
console.log(JSON.stringify({
inputPayload: payload.json,
inputStatus: payload.status,
fallsthroughTo: fallbackReturn,
arithmeticUsingNullishCoalescing: arithmeticSafe,
arithmeticUsingTruthiness: computed,
}))
}
JSRepository: yashdev9274/supercli
Length of output: 971
Enforce fallbacks for non-OK responses and invalid payload fields.
safeFetchJson currently returns any parsed JSON for 5xx/redirect responses, e.g. count: "error", while getStats() coerces that to 1 + 100. Add if (!res.ok) return fallback before res.json() and validate each payload schema with Zod before the fallback arithmetic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/modules/stats/actions/index.ts` around lines 3 - 10, Update
safeFetchJson to return fallback when the fetch response is not OK before
parsing JSON. In getStats, validate each fetched payload with the existing or
appropriate Zod schemas before performing fallback arithmetic, so invalid fields
such as string counts use the documented fallback instead of being coerced.
Source: Coding guidelines
| const serverUrl = process.env.TERMINAL_SERVER_URL || "http://localhost:3004" | ||
| const [userStats, npmResponse, githubResponse] = await Promise.all([ | ||
| safeFetchJson<{ count?: number }>(`${serverUrl}/api/data/users/count`, { count: 0 }), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align the terminal-server URL fallback.
getStats defaults to http://localhost:3004, but apps/supercode-cli/server/src/index.ts defaults PORT to 10000 at Line 27. Without TERMINAL_SERVER_URL, this request misses the terminal server and the stats page silently reports fallback users. Reuse the same URL resolver and fallback chain as apps/web/app/api/data/analytics/route.ts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/modules/stats/actions/index.ts` around lines 13 - 15, Update
getStats to resolve the terminal-server URL using the same resolver and fallback
chain as the analytics route, rather than defaulting directly to
http://localhost:3004. Reuse the existing shared URL-resolution logic so the
fallback aligns with the terminal server’s PORT=10000 default and requests the
correct users endpoint.
1 participant
Description
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
bun testpassesbun run typecheckpassesbun run lintpasses (if applicable)Checklist:
Summary by CodeRabbit
New Features
Bug Fixes