-
Notifications
You must be signed in to change notification settings - Fork 51
Daily branch 2025 09 23 #37
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
WalkthroughNormalizes entitlement checks to recognize normalized and legacy monthly/yearly plan keys, adds yearly billing support in UI and APIs, simplifies Convex auth provider and enables eager auth, adjusts dialog layout and dependency versions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as PricingDialog
participant Sel as BillingFrequencySelector
participant Hook as useUpgrade
participant API as POST /api/subscribe
User->>Sel: Toggle Monthly/Yearly
Sel-->>UI: onChange(frequency)
User->>UI: Click Upgrade (Pro/Ultra)
UI->>Hook: handleUpgrade(planKey based on frequency)
Hook->>API: Create checkout for planKey
API-->>Hook: Checkout URL / status
Hook-->>UI: Result
UI-->>User: Redirect to checkout or show error
sequenceDiagram
autonumber
participant Client as Client App
participant ENT as GET /api/entitlements
participant S as Auth/Storage
participant GS as GlobalState
Client->>ENT: Request entitlements
ENT->>S: Fetch user entitlements
S-->>ENT: { keys: [...normalized & legacy...] }
ENT->>ENT: Determine subscription (ultra > pro > free)
ENT-->>Client: { subscription, entitlements }
Client->>GS: Update global subscription from keys
GS-->>Client: Derived state updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/auth/get-user-id.ts (1)
33-40: Fix JSDoc to match the actual return shape (remove isPro).Update the description and @returns to avoid misleading callers.
Apply this diff:
- * Get the current user ID and pro status from the authenticated session + * Get the current user ID and subscription tier from the authenticated session @@ - * @returns Promise<{ userId: string; isPro: boolean; subscription: SubscriptionTier }> - Object with userId, isPro, and subscription + * @returns Promise<{ userId: string; subscription: SubscriptionTier }> - Object with userId and subscription tierapp/components/PricingDialog.tsx (1)
228-248: Unauthenticated users can trigger upgrade flow on Ultra. Redirect to sign-in instead.Unlike Pro, Ultra always calls handleUpgradeClick even when user is null.
Apply this diff:
const getUltraButtonConfig = () => { if (user && !isCheckingProPlan && subscription === "ultra") { return { text: "Current Plan", disabled: true, className: "opacity-50 cursor-not-allowed", variant: "secondary" as const, }; } - return { - text: user ? "Get Ultra" : "Get Ultra", - disabled: upgradeLoading, - className: "", - variant: "default" as const, - onClick: () => - handleUpgradeClick( - isYearly ? "ultra-yearly-plan" : "ultra-monthly-plan", - ), - loading: upgradeLoading, - } as const; + if (user) { + return { + text: "Get Ultra", + disabled: upgradeLoading, + className: "", + variant: "default" as const, + onClick: () => + handleUpgradeClick( + isYearly ? "ultra-yearly-plan" : "ultra-monthly-plan", + ), + loading: upgradeLoading, + } as const; + } + return { + text: "Get Ultra", + disabled: false, + className: "", + variant: "default" as const, + onClick: handleSignIn, + } as const; };
🧹 Nitpick comments (10)
app/api/subscribe/route.ts (1)
16-21: Strongly type plan keys via a literal tuple and a type guard; avoid the cast.Prevents drift between runtime and type union, and keeps the validator single‑sourced.
Apply this diff to derive the type from the values and keep Set membership checks fast:
- const allowedPlans = new Set([ - "pro-monthly-plan", - "ultra-monthly-plan", - "pro-yearly-plan", - "ultra-yearly-plan", - ]); + const ALLOWED_PLANS = [ + "pro-monthly-plan", + "ultra-monthly-plan", + "pro-yearly-plan", + "ultra-yearly-plan", + ] as const; + type PlanKey = typeof ALLOWED_PLANS[number]; + const allowedPlans = new Set<PlanKey>(ALLOWED_PLANS);- typeof requestedPlan === "string" && allowedPlans.has(requestedPlan) - ? (requestedPlan as - | "pro-monthly-plan" - | "ultra-monthly-plan" - | "pro-yearly-plan" - | "ultra-yearly-plan") + isPlanKey(requestedPlan) && allowedPlans.has(requestedPlan) + ? requestedPlan : "pro-monthly-plan";Add this helper near the declarations (outside the changed lines):
function isPlanKey(v: unknown): v is PlanKey { return typeof v === "string" && (ALLOWED_PLANS as readonly string[]).includes(v); }Please also confirm the corresponding Stripe price lookup_keys exist for each new plan key in your Stripe dashboard.
Also applies to: 24-28
convex/fileStorage.ts (2)
20-26: Avoid .collect() for counts; prefer async iteration.This aligns with Convex best practices and avoids materializing arrays.
- const files = await ctx.db - .query("files") - .withIndex("by_user_id", (q) => q.eq("user_id", args.userId)) - .collect(); - - return files.length; + const q = ctx.db + .query("files") + .withIndex("by_user_id", (q) => q.eq("user_id", args.userId)); + let count = 0; + for await (const _ of q) { + count++; + } + return count;
75-84: Centralize paid‑entitlement keys and simplify the check.Same keys appear across files; suggest a shared constant to prevent drift.
- const isPaid = - entitlements.includes("pro-plan") || - entitlements.includes("ultra-plan") || - entitlements.includes("pro-monthly-plan") || - entitlements.includes("ultra-monthly-plan") || - entitlements.includes("pro-yearly-plan") || - entitlements.includes("ultra-yearly-plan"); + const isPaid = entitlements.some((e) => PAID_ENTITLEMENTS.has(e));Add once (e.g., in a shared module like lib/billing/entitlements.ts) and import here:
export const PAID_ENTITLEMENTS = new Set([ "pro-plan", "ultra-plan", "pro-monthly-plan", "ultra-monthly-plan", "pro-yearly-plan", "ultra-yearly-plan", ] as const);app/hooks/useUpgrade.ts (1)
10-15: Avoid duplicating plan key unions; reuse a shared PlanKey type.Keeps UI/server in sync as new variants are added.
Create and export a PlanKey type (e.g., from lib/billing/entitlements.ts) derived from a single source array and import it here:
export const PLAN_KEYS = [ "pro-monthly-plan", "ultra-monthly-plan", "pro-yearly-plan", "ultra-yearly-plan", ] as const; export type PlanKey = typeof PLAN_KEYS[number];app/contexts/GlobalState.tsx (1)
184-201: OK, and consider Set + shared constants to DRY/optimize.The logic is correct. To avoid repeating keys across files and reduce multiple includes calls:
- const hasUltra = - entitlements.includes("ultra-plan") || - entitlements.includes("ultra-monthly-plan") || - entitlements.includes("ultra-yearly-plan"); - const hasPro = - entitlements.includes("pro-plan") || - entitlements.includes("pro-monthly-plan") || - entitlements.includes("pro-yearly-plan"); + const e = new Set(entitlements); + const hasUltra = ["ultra-plan","ultra-monthly-plan","ultra-yearly-plan"].some((k) => e.has(k)); + const hasPro = ["pro-plan","pro-monthly-plan","pro-yearly-plan"].some((k) => e.has(k));Prefer importing shared constants (e.g., ULTRA_KEYS/PRO_KEYS) from a single module used by server and client.
app/api/entitlements/route.ts (1)
71-86: Normalization looks good; centralize subscription derivation.To keep server/client logic consistent, move the ultra/pro key sets and deriveSubscription(entitlements) into a shared module and import here.
app/components/PricingDialog.tsx (1)
152-158: Centralize plan key union to avoid drift across files.Define a shared PlanKey type (and possibly constants) used by hooks/routes/UI to keep keys in sync.
Example (in a shared module, e.g., lib/pricing/planKeys.ts):
export type PlanKey = | "pro-monthly-plan" | "ultra-monthly-plan" | "pro-yearly-plan" | "ultra-yearly-plan"; export const PLAN_KEYS = { PRO_MONTHLY: "pro-monthly-plan", PRO_YEARLY: "pro-yearly-plan", ULTRA_MONTHLY: "ultra-monthly-plan", ULTRA_YEARLY: "ultra-yearly-plan", } as const;Then import PlanKey/PLAN_KEYS here and in useUpgrade, routes, etc.
app/components/BillingFrequencySelector.tsx (2)
20-23: Remove unused segmentedRef.segmentedRef is never read; drop it to reduce noise.
Apply this diff:
- const segmentedRef = React.useRef<HTMLDivElement | null>(null); @@ - ref={segmentedRef} className={`relative inline-flex items-center rounded-full border border-border bg-background p-1 ${className}`}Also applies to: 69-69
5-12: Consider exporting the BillingFrequency type and props.Helps reuse consistent typing across components/hooks.
Apply this diff:
-type BillingFrequency = "monthly" | "yearly"; +export type BillingFrequency = "monthly" | "yearly"; @@ -interface BillingFrequencySelectorProps { +export interface BillingFrequencySelectorProps {lib/auth/get-user-id.ts (1)
41-46: Call sites OK; update JSDoc (isPro removed)
- Repo search shows callers updated — only call found: app/api/chat/route.ts:78 now destructures
{ userId, subscription }.- lib/auth/get-user-id.ts JSDoc still mentions
isPro— remove/update that doc to match the new return type.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
app/api/entitlements/route.ts(1 hunks)app/api/subscribe/route.ts(1 hunks)app/components/BillingFrequencySelector.tsx(1 hunks)app/components/CustomizeHackerAIDialog.tsx(1 hunks)app/components/PricingDialog.tsx(8 hunks)app/contexts/GlobalState.tsx(1 hunks)app/hooks/useUpgrade.ts(1 hunks)components/ConvexClientProvider.tsx(5 hunks)convex/fileStorage.ts(1 hunks)lib/auth/get-user-id.ts(1 hunks)middleware.ts(1 hunks)package.json(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id helper type from ./_generated/dataModel to type document IDs (e.g., Id<'users'>) instead of string
When defining Record types, specify key and value types matching validators (e.g., Record<Id<'users'>, string>)
Be strict with types for document IDs; prefer Id<'table'> over string in function args and variables
Useas constfor string literals in discriminated unions
Declare arrays with explicit generic type: const arr: Array = [...]
Declare records with explicit generic types: const record: Record<KeyType, ValueType> = {...}
Files:
middleware.tsapp/components/BillingFrequencySelector.tsxapp/hooks/useUpgrade.tsapp/contexts/GlobalState.tsxapp/components/PricingDialog.tsxconvex/fileStorage.tslib/auth/get-user-id.tscomponents/ConvexClientProvider.tsxapp/components/CustomizeHackerAIDialog.tsxapp/api/subscribe/route.tsapp/api/entitlements/route.ts
package.json
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
Add @types/node to devDependencies when using Node.js built-in modules
Files:
package.json
convex/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.ts: Always use the new Convex function syntax (query/mutation/action objects with args/returns/handler) when defining Convex functions
When a function returns null, include returns: v.null() and return null explicitly
Use internalQuery/internalMutation/internalAction for private functions callable only by other Convex functions; do not expose sensitive logic via public query/mutation/action
Use query/mutation/action only for public API functions
Do not try to register functions via the api or internal objects
Always include argument and return validators for all Convex functions (query/internalQuery/mutation/internalMutation/action/internalAction)
In JS implementations, functions without an explicit return value implicitly return null
Use ctx.runQuery from queries/mutations/actions to call a query
Use ctx.runMutation from mutations/actions to call a mutation
Use ctx.runAction from actions to call an action
Only call an action from another action when crossing runtimes (e.g., V8 to Node); otherwise extract shared helper code
Minimize calls from actions to queries/mutations to avoid race conditions from splitting transactions
Pass FunctionReference values (from api/internal) to ctx.runQuery/ctx.runMutation/ctx.runAction; do not pass function implementations
When calling a function in the same file via ctx.run*, add an explicit return type annotation at the call site to avoid TS circularity
Use the generated api object for public functions and internal object for internal functions from convex/_generated/api.ts
Respect file-based routing for function references: e.g., convex/example.ts export f -> api.example.f; nested paths map to dot-separated namespaces
For paginated queries use paginationOptsValidator in args and .paginate(args.paginationOpts) on a query
v.bigint() is deprecated; use v.int64() for signed 64-bit integers
Use v.record(keys, values) for record-like data; v.map() and v.set() are not supported
For full-text search, use withSearchIndex("ind...
Files:
convex/fileStorage.ts
🧠 Learnings (1)
📚 Learning: 2025-08-29T13:33:09.937Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.937Z
Learning: Applies to convex/**/*.ts : Use ctx.storage.getUrl(fileId) to obtain signed URLs; it returns null if the file does not exist
Applied to files:
convex/fileStorage.ts
🧬 Code graph analysis (1)
lib/auth/get-user-id.ts (1)
types/chat.ts (1)
SubscriptionTier(6-6)
🔇 Additional comments (11)
middleware.ts (1)
4-4: eagerAuth enabled — verify perf and route coverage.Eager auth can add a per‑request roundtrip. Given the matcher excludes static assets, impact should be small, but please verify TTFB on key pages and that all public unauthenticated paths are listed.
package.json (1)
78-78: Good: @types/node present per guidelines.Dev deps include @types/node, aligning with usage of Node APIs.
convex/fileStorage.ts (1)
116-118: LGTM: using ctx.storage.getUrl is correct.Matches Convex guidance for signed URLs; returns null when absent.
app/components/CustomizeHackerAIDialog.tsx (1)
146-156: LGTM: scrollable content with persistent footer.The flex/overflow structure improves usability on small screens; footer actions remain accessible.
Also applies to: 287-303
lib/auth/get-user-id.ts (1)
62-71: Good normalization of entitlement keys.Handling both normalized and legacy keys via hasUltra/hasPro makes subscription derivation robust.
components/ConvexClientProvider.tsx (2)
26-31: Auth adapter shape looks correct for ConvexProviderWithAuth.isLoading, isAuthenticated, and fetchAccessToken satisfy expected contract.
Also applies to: 53-57
12-23: Verify consumers: provider props simplified (breaking change)components/ConvexClientProvider.tsx now exposes a provider that takes no props (e.g., expectAuth removed). Search for and update any consumers that previously passed expectAuth or other provider props — no occurrences found in this repository; confirm external/monorepo consumers are updated.
app/components/PricingDialog.tsx (2)
139-143: LGTM: Clean yearly/monthly state handling.Local isYearly with a small handler keeps the UI logic simple.
276-282: Verify discount copy vs. prices (17% label vs. 15% on Pro).Selector says “Save 17%”, but Pro yearly shows 17 vs 20 (~15% off) while Ultra shows 166 vs 200 (~17% off). Align copy and/or prices.
Would you like to standardize on 17% off across tiers (e.g., Pro at 16.6 or 16.5) or adjust the copy to “Save up to 17%”?
Also applies to: 301-301, 317-317
app/components/BillingFrequencySelector.tsx (2)
31-37: Keyboard handling: confirm no double-toggling with native radio behavior.onKeyDown at the container prevents default; ensure this doesn’t conflict with native radio navigation when an input has focus.
Manual check:
- Focus a radio input and press ArrowRight/Left; verify the selection changes once per keypress and focus remains within the group.
39-51: Indicator measurement logic looks sound.The delayed update and resize handler keep the indicator aligned with the selected option.
Also applies to: 53-65
| planKey?: | ||
| | "pro-monthly-plan" | ||
| | "ultra-monthly-plan" | ||
| | "pro-yearly-plan" | ||
| | "ultra-yearly-plan", | ||
| e?: React.MouseEvent<HTMLButtonElement | HTMLDivElement>, | ||
| ) => { |
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.
Fix TS error: React.MouseEvent type used without importing React types.
This breaks type‑checking in a .ts file.
Add a type‑only import:
+import type React from "react";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| planKey?: | |
| | "pro-monthly-plan" | |
| | "ultra-monthly-plan" | |
| | "pro-yearly-plan" | |
| | "ultra-yearly-plan", | |
| e?: React.MouseEvent<HTMLButtonElement | HTMLDivElement>, | |
| ) => { | |
| import type React from "react"; | |
| planKey?: | |
| | "pro-monthly-plan" | |
| | "ultra-monthly-plan" | |
| | "pro-yearly-plan" | |
| | "ultra-yearly-plan", | |
| e?: React.MouseEvent<HTMLButtonElement | HTMLDivElement>, | |
| ) => { |
🤖 Prompt for AI Agents
In app/hooks/useUpgrade.ts around lines 10-16, the function signature uses
React.MouseEvent without importing React types; add a type-only import for
MouseEvent from 'react' at the top of the file and update the parameter type to
use that imported MouseEvent<HTMLButtonElement | HTMLDivElement> so the file
passes TS type-checking without importing the whole React namespace.
| export function ConvexClientProvider({ children }: { children: ReactNode }) { | ||
| const [convex] = useState(() => { | ||
| return new ConvexReactClient(process.env.NEXT_PUBLIC_CONVEX_URL!, { | ||
| expectAuth, | ||
| }); | ||
| return new ConvexReactClient(process.env.NEXT_PUBLIC_CONVEX_URL!); | ||
| }); |
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.
Guard against missing NEXT_PUBLIC_CONVEX_URL.
Non-null assertion may hide a misconfig and crash at runtime. Fail fast with a clear error.
Apply this diff:
export function ConvexClientProvider({ children }: { children: ReactNode }) {
const [convex] = useState(() => {
- return new ConvexReactClient(process.env.NEXT_PUBLIC_CONVEX_URL!);
+ const url = process.env.NEXT_PUBLIC_CONVEX_URL;
+ if (!url) {
+ throw new Error("Missing NEXT_PUBLIC_CONVEX_URL for ConvexReactClient.");
+ }
+ return new ConvexReactClient(url);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function ConvexClientProvider({ children }: { children: ReactNode }) { | |
| const [convex] = useState(() => { | |
| return new ConvexReactClient(process.env.NEXT_PUBLIC_CONVEX_URL!, { | |
| expectAuth, | |
| }); | |
| return new ConvexReactClient(process.env.NEXT_PUBLIC_CONVEX_URL!); | |
| }); | |
| export function ConvexClientProvider({ children }: { children: ReactNode }) { | |
| const [convex] = useState(() => { | |
| const url = process.env.NEXT_PUBLIC_CONVEX_URL; | |
| if (!url) { | |
| throw new Error("Missing NEXT_PUBLIC_CONVEX_URL for ConvexReactClient."); | |
| } | |
| return new ConvexReactClient(url); | |
| }); |
🤖 Prompt for AI Agents
In components/ConvexClientProvider.tsx around lines 12 to 15, the code uses a
non-null assertion on process.env.NEXT_PUBLIC_CONVEX_URL which can hide
misconfiguration and crash; change the initializer to first read
process.env.NEXT_PUBLIC_CONVEX_URL into a const, check if it's undefined or
empty and if so throw a clear Error (e.g. "NEXT_PUBLIC_CONVEX_URL is not set"),
then create the ConvexReactClient with that validated value inside the useState
initializer so the app fails fast with an explicit message instead of a runtime
crash.
Summary by CodeRabbit
New Features
Improvements
UI
Chores