issue 112#125
Open
armaansharma-dev wants to merge 9 commits intoyb175:mainfrom
Open
Conversation
…, separate schemas, validation
…, separate schemas, validation
…marks, Saved page, loading states, page transitions, professional redesign
Contributor
📝 WalkthroughWalkthroughThis PR introduces a comprehensive UI component library with design tokens, refactors routing with error handling and notification providers, adds bookmark functionality, implements resume checking and pagination, and updates multiple pages to use new shared layout primitives and toast-based notifications. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
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🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/services/resume.ts (1)
8-13:⚠️ Potential issue | 🟠 MajorDo not treat failed uploads as successful responses.
Line 13 returns parsed JSON without anres.okcheck, so 4xx/5xx upload failures can flow through as success paths.As per coding guidelines, "ONLY flag issues that fall into one of these HIGH-IMPACT categories: 3. Runtime crashes ... unhandled promise rejections in async handlers".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/services/resume.ts` around lines 8 - 13, The current upload flow returns parsed JSON unconditionally after calling fetchWithAuth, which lets 4xx/5xx responses be treated as success; in the function that calls fetchWithAuth (look for the method that posts to "/resume" where res is awaited), check res.ok and, if false, read the error payload (res.text() or res.json()) and throw a descriptive error including res.status and the body before returning the success payload; replace the direct return of res.json() with this conditional check so callers receive a rejected promise on failed uploads.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/Landing/CTAButton.tsx`:
- Around line 24-45: The effect can leave stale resume state across
sign-out/sign-in and race with the async checkResume call; update the hook to
reset hasResume immediately on sign-out and cancel/ignore stale async results:
when isSignedIn becomes false call setHasResume(false) (and
setIsLoading(false)), and inside the effect that runs when isSignedIn/isLoaded
create a cancellation flag or requestId (e.g., local let cancelled = false or a
ref) before calling checkResumeExists(fetchRef.current), then only call
setHasResume(exists) and setIsLoading(false) if not cancelled and the current
sign-in session still applies; also return a cleanup that sets cancelled = true
to prevent stale updates from checkResumeExists from overwriting the new
session’s state.
In `@frontend/src/components/ui/ErrorBoundary.tsx`:
- Line 44: Replace the direct rendering of the exception text in the
ErrorBoundary fallback UI (the expression this.state.error?.message inside the
ErrorBoundary component) with a generic user-facing message, and ensure the
actual error is recorded via console.error or your telemetry/logger (e.g., call
console.error(this.state.error) or send to reportError) so internal details are
not exposed to users; update the renderFallback logic to show a fixed safe
string like "Something went wrong" while keeping the logging/reporting of
this.state.error intact.
In `@frontend/src/components/ui/WelcomeTour.tsx`:
- Around line 4-8: The TourStep interface declares position?: 'top' | 'bottom' |
'left' | 'right' but the WelcomeTour component's rendering logic (e.g., the
renderStep/getPlacement code path) currently treats every non-'top' value as
'bottom', so either narrow the TourStep.position type to only 'top' | 'bottom'
to reflect implemented behavior, or implement the missing 'left' and 'right'
placements in the WelcomeTour rendering (update the function that maps
step.position to tooltip/popover placement and the CSS/positioning logic for
'left' and 'right' branches) so the public type matches actual rendering; adjust
types and unit/UI tests accordingly.
In `@frontend/src/pages/JobPage.tsx`:
- Line 1: The resume-check effect in JobPage doesn't react to auth changes
because fetchWithAuth from useApi is recreated each render; memoize
fetchWithAuth inside useApi using useCallback (so it only changes when its true
dependencies change), then update the resume effect to depend on [fetchWithAuth]
(removing any ref-workaround) so hasResume updates on auth/token changes without
causing repeated API calls; target the fetchWithAuth export in useApi and the
resume effect that sets hasResume in JobPage.
---
Outside diff comments:
In `@frontend/src/services/resume.ts`:
- Around line 8-13: The current upload flow returns parsed JSON unconditionally
after calling fetchWithAuth, which lets 4xx/5xx responses be treated as success;
in the function that calls fetchWithAuth (look for the method that posts to
"/resume" where res is awaited), check res.ok and, if false, read the error
payload (res.text() or res.json()) and throw a descriptive error including
res.status and the body before returning the success payload; replace the direct
return of res.json() with this conditional check so callers receive a rejected
promise on failed uploads.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b338c4bc-3064-4d12-b749-5540f9d74dcf
⛔ Files ignored due to path filters (1)
frontend/src/assets/logo1.svgis excluded by!**/*.svg
📒 Files selected for processing (27)
frontend/src/App.tsxfrontend/src/components/Header.tsxfrontend/src/components/Jobs/JobCard.tsxfrontend/src/components/Jobs/JobList.tsxfrontend/src/components/Landing/CTAButton.tsxfrontend/src/components/Landing/Hero.tsxfrontend/src/components/Profile/ResumeUpdateSection.tsxfrontend/src/components/ui/Alert.tsxfrontend/src/components/ui/Badge.tsxfrontend/src/components/ui/BookmarksProvider.tsxfrontend/src/components/ui/Button.tsxfrontend/src/components/ui/Card.tsxfrontend/src/components/ui/ErrorBoundary.tsxfrontend/src/components/ui/Input.tsxfrontend/src/components/ui/PageLayout.tsxfrontend/src/components/ui/PageTransition.tsxfrontend/src/components/ui/Skeleton.tsxfrontend/src/components/ui/Toast.tsxfrontend/src/components/ui/ToastProvider.tsxfrontend/src/components/ui/WelcomeTour.tsxfrontend/src/components/ui/index.tsfrontend/src/pages/JobPage.tsxfrontend/src/pages/LandingPage.tsxfrontend/src/pages/ProfilePage.tsxfrontend/src/pages/SavedPage.tsxfrontend/src/services/resume.tsfrontend/tailwind.config.js
Comment on lines
+24
to
+45
| useEffect(() => { | ||
| if (!isLoaded) return; | ||
|
|
||
| if (!isSignedIn) { | ||
| setIsLoading(false); | ||
| return; | ||
| } | ||
|
|
||
| const checkResume = async () => { | ||
| try { | ||
| const exists = await checkResumeExists(fetchRef.current); | ||
| setHasResume(exists); | ||
| } catch (err) { | ||
| console.error("Failed to check resume:", err); | ||
| setHasResume(false); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| checkResume(); | ||
| }, [isSignedIn, isLoaded]); |
Contributor
There was a problem hiding this comment.
Reset and cancel auth-derived resume checks.
Without clearing hasResume and ignoring stale async results here, a sign-out/sign-in sequence can reuse the previous session's resume state and send the next user to the wrong page.
Suggested fix
useEffect(() => {
if (!isLoaded) return;
if (!isSignedIn) {
+ setHasResume(null);
setIsLoading(false);
return;
}
+ setHasResume(null);
+ setIsLoading(true);
+ let cancelled = false;
+
const checkResume = async () => {
try {
const exists = await checkResumeExists(fetchRef.current);
- setHasResume(exists);
+ if (!cancelled) setHasResume(exists);
} catch (err) {
console.error("Failed to check resume:", err);
- setHasResume(false);
+ if (!cancelled) setHasResume(false);
} finally {
- setIsLoading(false);
+ if (!cancelled) setIsLoading(false);
}
};
- checkResume();
+ void checkResume();
+
+ return () => {
+ cancelled = true;
+ };
}, [isSignedIn, isLoaded]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/Landing/CTAButton.tsx` around lines 24 - 45, The
effect can leave stale resume state across sign-out/sign-in and race with the
async checkResume call; update the hook to reset hasResume immediately on
sign-out and cancel/ignore stale async results: when isSignedIn becomes false
call setHasResume(false) (and setIsLoading(false)), and inside the effect that
runs when isSignedIn/isLoaded create a cancellation flag or requestId (e.g.,
local let cancelled = false or a ref) before calling
checkResumeExists(fetchRef.current), then only call setHasResume(exists) and
setIsLoading(false) if not cancelled and the current sign-in session still
applies; also return a cleanup that sets cancelled = true to prevent stale
updates from checkResumeExists from overwriting the new session’s state.
| Something went wrong | ||
| </h1> | ||
| <p className="text-text-secondary mb-6"> | ||
| {this.state.error?.message || 'An unexpected error occurred'} |
Contributor
There was a problem hiding this comment.
Avoid rendering raw exception messages to users.
Line 44 exposes error.message directly in the fallback UI and can leak internal details, so render a generic message instead.
As per coding guidelines, "ONLY flag issues that fall into one of these HIGH-IMPACT categories: 1. Security vulnerabilities (hardcoded secrets, tokens, XSS risks via dangerouslySetInnerHTML, exposed credentials)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/ui/ErrorBoundary.tsx` at line 44, Replace the direct
rendering of the exception text in the ErrorBoundary fallback UI (the expression
this.state.error?.message inside the ErrorBoundary component) with a generic
user-facing message, and ensure the actual error is recorded via console.error
or your telemetry/logger (e.g., call console.error(this.state.error) or send to
reportError) so internal details are not exposed to users; update the
renderFallback logic to show a fixed safe string like "Something went wrong"
while keeping the logging/reporting of this.state.error intact.
Comment on lines
+4
to
+8
| interface TourStep { | ||
| target: string; | ||
| title: string; | ||
| description: string; | ||
| position?: 'top' | 'bottom' | 'left' | 'right'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,140p' frontend/src/components/ui/WelcomeTour.tsx
rg -n -C4 '<WelcomeTour\b|WelcomeTour\(' frontend/srcRepository: yb175/CVPilot
Length of output: 4855
🏁 Script executed:
#!/bin/bash
# Check for any imports of WelcomeTour
rg -n 'import.*WelcomeTour|from.*WelcomeTour' frontend/src
# Check if it's exported in barrel files
fd -e ts -e tsx | xargs grep -l 'WelcomeTour' 2>/dev/null | head -20Repository: yb175/CVPilot
Length of output: 219
Narrow position or implement the missing placements.
Lines 4-8 allow 'left' and 'right', but line 98 sends every non-'top' value through the bottom branch. The public type promises behavior this component does not render.
Suggested fix
interface TourStep {
target: string;
title: string;
description: string;
- position?: 'top' | 'bottom' | 'left' | 'right';
+ position?: 'top' | 'bottom';
}📝 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.
Suggested change
| interface TourStep { | |
| target: string; | |
| title: string; | |
| description: string; | |
| position?: 'top' | 'bottom' | 'left' | 'right'; | |
| interface TourStep { | |
| target: string; | |
| title: string; | |
| description: string; | |
| position?: 'top' | 'bottom'; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/ui/WelcomeTour.tsx` around lines 4 - 8, The TourStep
interface declares position?: 'top' | 'bottom' | 'left' | 'right' but the
WelcomeTour component's rendering logic (e.g., the renderStep/getPlacement code
path) currently treats every non-'top' value as 'bottom', so either narrow the
TourStep.position type to only 'top' | 'bottom' to reflect implemented behavior,
or implement the missing 'left' and 'right' placements in the WelcomeTour
rendering (update the function that maps step.position to tooltip/popover
placement and the CSS/positioning logic for 'left' and 'right' branches) so the
public type matches actual rendering; adjust types and unit/UI tests
accordingly.
| @@ -1,97 +1,160 @@ | |||
| import { useState } from "react"; | |||
| import { useState, useEffect, useRef } from "react"; | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C4 'export function useApi|function useApi|const useApi|fetchWithAuth' frontend/srcRepository: yb175/CVPilot
Length of output: 9094
🏁 Script executed:
cat -n frontend/src/lib/fetcher.tsRepository: yb175/CVPilot
Length of output: 1131
🏁 Script executed:
sed -n '15,50p' frontend/src/pages/JobPage.tsxRepository: yb175/CVPilot
Length of output: 1235
🏁 Script executed:
sed -n '8,50p' frontend/src/components/Landing/CTAButton.tsxRepository: yb175/CVPilot
Length of output: 1143
The resume check won't rerun if auth state changes mid-session, but the suggested fix would cause infinite API calls.
The resume effect has an empty dependency array while fetchWithAuth is unstable (recreated every render from useApi()). If the token refreshes or auth state changes, hasResume won't update. However, adding fetchWithAuth as a dependency would trigger the effect on every render (new function every render), flooding the API with duplicate requests.
The correct fix is to memoize fetchWithAuth in useApi() with useCallback first:
export const useApi = () => {
const { getToken } = useAuth();
- const fetchWithAuth = async (
+ const fetchWithAuth = useCallback(async (
endpoint: string,
options: RequestInit = {}
) => {
// ... implementation
- };
+ }, [getToken]);
return { fetchWithAuth };
};Then the effect can safely depend on [fetchWithAuth] without the ref workaround.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/JobPage.tsx` at line 1, The resume-check effect in JobPage
doesn't react to auth changes because fetchWithAuth from useApi is recreated
each render; memoize fetchWithAuth inside useApi using useCallback (so it only
changes when its true dependencies change), then update the resume effect to
depend on [fetchWithAuth] (removing any ref-workaround) so hasResume updates on
auth/token changes without causing repeated API calls; target the fetchWithAuth
export in useApi and the resume effect that sets hasResume in JobPage.
Collaborator
Author
|
You are acting as a senior backend + system design reviewer. Your task is to analyze the current repository and convert the MVP goal into a well-structured execution plan. 🎯 Objective Backend APIs (existing or partially built) 📌 What you need to do
Each sub-issue must include: Title Setup API route for X
Missing endpoints Backend:
Frontend → API → Service → DB → Response → UI Keep it short but clear.
No auth complexity unless already present ✅ Whether current structure supports MVP Scraping / data fetching is deterministic 🚫 Avoid |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me first explore the full repository structure before generating the plan. 🏁 Script executed: Length of output: 2832 Good. Now let me read all the key files in one pass. 🏁 Script executed: Length of output: 39684
1. Codebase Summary
2. Sub-Issues List🔴 P0 — Blockers (MVP cannot deploy without these)SUB-1: Add
|
| # | Gap | Severity | Location |
|---|---|---|---|
| 1 | No /jobs API — entire jobs page is mocked |
🔴 Critical | Backend missing entirely |
| 2 | No Job model in Prisma schema |
🔴 Critical | schema.prisma |
| 3 | Job matching logic doesn't exist | 🔴 Critical | No jobs.service.ts |
| 4 | datasource db missing url = env("DATABASE_URL") |
🔴 Critical | schema.prisma |
| 5 | syncClerkUser(userId, "") creates placeholder email users |
🟡 High | resumeController.ts Line 26 |
| 6 | originalFileName used in frontend but not in DB schema |
🟡 High | schema.prisma, ProfilePage.tsx |
| 7 | Resume 202 (parsing) not polled — user stuck | 🟡 High | JobPage.tsx, ProfilePage.tsx |
| 8 | Bookmarks only in localStorage — lost on new device/session | 🟡 Medium | BookmarksProvider.tsx |
| 9 | Refresh Jobs button in JobPage just fakes a delay with setTimeout — doesn't re-fetch |
🟡 Medium | JobPage.tsx |
| 10 | No scraping/ingestion pipeline connected to DB — jobs must be manually seeded | 🟡 Medium | Backend |
4. Integration Flow
User uploads resume
│
▼
POST /resume ──► Cloudinary (store file)
│
▼
parsingTrigger() [fire-and-forget]
├── Extract text (PDF/DOCX)
├── Gemini LLM → ParsedResume JSON
├── Fallback rule-based parser
└── DB: Resume.parsedData = { skills, experience, ... }
│
▼ (user sets prefs meanwhile)
POST /preferences ──► DB: UserPreferences { seniority, locationPreferences }
│
▼
Frontend polls GET /resume (202 → 200)
│
▼ (parsedData ready)
GET /jobs [NEW] ──► jobs.service.ts
├── Read Resume.parsedData.skills
├── Read UserPreferences.seniority + locationPreferences
├── Query Job table with filters
└── Score by skill overlap → sorted list
│
▼
Frontend JobsPage renders real matched jobs
5. Final Verdict
✅ What already supports MVP
- Full resume upload + async AI parsing pipeline is production-grade
- Preferences CRUD is complete end-to-end
- Auth (Clerk) is solid and consistent
- Frontend service layer (
fetchWithAuth) is correctly abstracted - UI is complete and polished
⚠️ Risks / Blockers
/jobsAPI is the single largest blocker — without it, the core value prop (job matching) doesn't existdatasource dbmissingurl— may cause silent failures in CI/CD migrations- Job scoring is trivial string matching — acceptable for MVP but set expectations accordingly
- Resume parsing is async (202) but the frontend doesn't wait → user may see "no jobs" immediately after upload
🧩 Must complete before deployment
| Priority | Task | Sub-Issue |
|---|---|---|
| 1 | Fix datasource db URL |
SUB-5 |
| 2 | Add Job model + migration |
SUB-1 |
| 3 | Seed jobs into DB | SUB-2 |
| 4 | Create GET /jobs endpoint |
SUB-3 |
| 5 | Connect JobsPage to /jobs |
SUB-4 |
| 6 | Fix syncClerkUser empty email |
SUB-6 |
| 7 | Add resume parsing poll (202→200) | SUB-8 |
SUB-7 (originalFileName) and SUB-9 (bookmarks backend) can be post-MVP.
Would you like me to open these as individual GitHub issues linked to a parent tracking issue, or generate the full implementation diffs for any specific sub-issue?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#112
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX Improvements