feat: Added a Basic Authentication System #123
Conversation
WalkthroughAdds Supabase authentication to the frontend: environment example and client, replaces mock login with supabase auth flows, adds signup/forgot/reset password pages, updates App routing and session handling, adjusts LandingPage navigation, and a minor editor config change. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as App.tsx
participant Client as supabase (client)
participant Pages as Router/Pages
rect #eef7ff Auth Init
User->>App: Load app
App->>Client: auth.getSession()
Client-->>App: session|null
App->>App: set isAuthenticated
App->>Client: onAuthStateChange(listener)
end
rect #f0ffef Login Flow
User->>Pages: Submit Login form
Pages->>Client: auth.signInWithPassword(email,password)
alt success
Client-->>Pages: session
Pages->>App: onLogin()
Pages->>User: Navigate to "/"
else error
Client-->>Pages: error
Pages->>User: Show error toast
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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: 6
🔭 Outside diff range comments (1)
.vimrc (1)
1-3:.vimrcshouldn’t live in the repo ‒ and the directive is invalid anyway
- Committing personal editor configuration clutters the project root and forces unrelated settings on every contributor. If shared editor settings are truly needed, place them under
./.editorconfig,./.vscode/, or a dedicatedtools/ide/folder—otherwise drop the file from the PR.syntax-highlighting=Trueis not a valid Vim command; the canonical directive is simplysyntax on(orsyntax enable).Recommended fix:
- syntax-highlighting=True + " Remove this file from the repository, or, if deliberate: + syntax on
🧹 Nitpick comments (7)
frontend/.env example (1)
1-3: Improve placeholder format for environment variables.The environment variable placeholders contain spaces which could cause configuration issues if copied directly. Consider using underscores or hyphens instead.
-VITE_SUPABASE_URL=YOUR SUPABASE URL +VITE_SUPABASE_URL=YOUR_SUPABASE_URL -VITE_SUPABASE_KEY=YOUR SUPABASE ANON KEY +VITE_SUPABASE_KEY=YOUR_SUPABASE_ANON_KEYfrontend/src/lib/supabaseClient.tsx (1)
1-9: Consider changing file extension from .tsx to .ts.This file contains no JSX elements, so the
.tsextension would be more appropriate than.tsx.frontend/src/App.tsx (1)
84-91: Fix formatting and spacing consistency.The signup route formatting is inconsistent with the login route above it.
<Route path="/signup" - element= {isAuthenticated ? ( - <Navigate to="/" replace /> - ) : ( - <SignUpPage/> - )} + element={ + isAuthenticated ? ( + <Navigate to="/" replace /> + ) : ( + <SignUpPage /> + ) + } />frontend/src/components/pages/SignUpPage.tsx (4)
23-25: Remove unused interface.The
LoginPagePropsinterface is not used in this SignUpPage component and appears to be leftover from copying the LoginPage component.-interface LoginPageProps { - onLogin: () => void; -}
107-112: Use appropriate icon for username field.The username field is using the Mail icon, which is semantically incorrect and confusing for users.
+import { + Settings, + Mail, + Lock, + LogIn, + User +} from 'lucide-react'; <InputField - icon={Mail} + icon={User} name="name" placeholder="Username" required />
191-197: Remove unnecessary state update before navigation.Setting
mailPageto false before navigating away is unnecessary since the component will unmount.<button type="button" - onClick={() => { setMailPage(false); navigate('/login') }} + onClick={() => navigate('/login')} className="mt-4 px-6 py-2 bg-green-500 hover:bg-green-600 text-white rounded-lg font-medium transition-colors" > Back to Sign In </button>
105-134: Consider adding client-side form validation for better UX.Adding real-time validation feedback would improve user experience by catching errors before form submission.
Consider implementing client-side validation with visual feedback:
const [errors, setErrors] = useState<{[key: string]: string}>({}); const validateField = (name: string, value: string) => { const newErrors = { ...errors }; switch (name) { case 'email': const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; if (!emailRegex.test(value)) { newErrors.email = 'Please enter a valid email address'; } else { delete newErrors.email; } break; case 'password': if (value.length < 6) { newErrors.password = 'Password must be at least 6 characters'; } else { delete newErrors.password; } break; } setErrors(newErrors); };Then add
onChangehandlers to InputField components and display error messages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
.vimrc(1 hunks)frontend/.env example(1 hunks)frontend/package.json(1 hunks)frontend/src/App.tsx(2 hunks)frontend/src/components/pages/LoginPage.tsx(3 hunks)frontend/src/components/pages/SignUpPage.tsx(1 hunks)frontend/src/lib/supabaseClient.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/components/pages/LoginPage.tsx (1)
frontend/src/lib/supabaseClient.tsx (1)
supabase(8-8)
🔇 Additional comments (7)
frontend/package.json (1)
13-13: Supabase dependency version is current and secureAs of August 6, 2025,
@supabase/supabase-js@^2.53.0is the latest release and has no publicly reported security vulnerabilities. Continue to monitor the Supabase GitHub repo and major CVE databases for future updates.• File
frontend/package.json, line 13:"@supabase/supabase-js": "^2.53.0",frontend/src/components/pages/LoginPage.tsx (3)
56-63: LGTM on form data extraction and authentication call.The FormData approach for extracting form values and the Supabase authentication call are implemented correctly.
93-93: LGTM on form field naming.Adding
nameattributes to form fields enables proper FormData extraction.
100-100: LGTM on form field naming.Adding
nameattributes to form fields enables proper FormData extraction.frontend/src/components/pages/SignUpPage.tsx (3)
29-40: LGTM!The AuthLayout component provides a clean, reusable wrapper with proper responsive design and smooth animations.
42-52: LGTM!The InputField component is well-designed with proper TypeScript typing, icon integration, and consistent styling. Good use of prop spreading for flexibility.
55-58: LGTM!Clean component setup with appropriate state management and hook usage.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
frontend/src/components/pages/ResetPasswrdPage.tsx (4)
83-85: Use strict equality and fix grammarMinor correctness and UX polish.
- if (password != repassword) { - toast.error("Passwords doesn't match. Try Again"); + if (password !== repassword) { + toast.error("Passwords do not match. Try again."); return; }
88-95: Optional: confirm success to the user before redirectA quick feedback toast improves UX.
- if (error) { + if (error) { toast.error(error.message || "An unknown error occurred!"); return; } - navigate("/"); + toast.success("Password updated successfully."); + navigate("/", { replace: true });
111-118: Client-side validation hints for password fieldsAdd
autoComplete="new-password"and a minimal length consistent with Supabase (default 6). This improves UX and browser behavior.<InputField icon={Key} type="password" name="password-1" className="mb-7" placeholder="New Password" + autoComplete="new-password" + minLength={6} required /> <InputField icon={Key} type="password" name="password-2" className="mb-7" placeholder="Reenter Password" + autoComplete="new-password" + minLength={6} required />Also applies to: 119-126
21-33: Deduplicate local Auth UI (DRY)
AuthLayoutandInputFieldare likely repeated across auth pages. Centralize them under a sharedcomponents/auth/module to reduce duplication and ensure consistent fixes/styles in one place.I can draft a shared
AuthLayoutandInputFieldwith full type safety and className merging. Want me to open a follow-up PR?Also applies to: 34-44
frontend/src/components/pages/ForgotPasswrdPage.tsx (2)
36-46: Caller-suppliedclassNameis lost
<InputField>spreads...propsbefore hard-codingclassName, so anyclassNamepassed by the caller (e.g.mb-7) is overridden. Merge the classes instead:- <input - {...props} - className="block w-full pl-10 pr-3 py-2 border ... " - /> + const { className, ...rest } = props; + <input + {...rest} + className={`block w-full pl-10 pr-3 py-2 border ... ${className ?? ''}`} + />
49-49: Typo in component/file name
ForgotPasswrdPageis missing an “o”. Standardising toForgotPasswordPageimproves discoverability and avoids future import mistakes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/src/App.tsx(2 hunks)frontend/src/components/pages/ForgotPasswrdPage.tsx(1 hunks)frontend/src/components/pages/LoginPage.tsx(4 hunks)frontend/src/components/pages/ResetPasswrdPage.tsx(1 hunks)frontend/src/components/pages/SignUpPage.tsx(1 hunks)frontend/src/lib/supabaseClient.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/src/lib/supabaseClient.tsx
- frontend/src/components/pages/SignUpPage.tsx
- frontend/src/App.tsx
- frontend/src/components/pages/LoginPage.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/components/pages/ForgotPasswrdPage.tsx (2)
frontend/src/lib/supabaseClient.tsx (1)
supabase(12-12)backend/app/core/config/settings.py (1)
Settings(8-51)
|
Hey @PinJinx , |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
frontend/src/components/pages/ResetPasswordPage.tsx (4)
67-90: Validatetype=recoverybefore setting sessionSupabase recovery links include
type=recovery. Guarding on this reduces accidental entry to this page with non-recovery links.- if (accessToken && refreshToken) { + const type = params.get('type'); + if (type === 'recovery' && accessToken && refreshToken) { (async () => { try { const { error } = await supabase.auth.setSession({ access_token: accessToken, refresh_token: refreshToken, }); if (error) { toast.error("Error setting session: " + error.message); navigate('/login', { replace: true }); } } catch { toast.error("Error setting session"); navigate('/login', { replace: true }); } finally { clearUrlHash(); } })(); } else { - toast.error("Access denied"); + toast.error("Invalid or expired recovery link"); navigate('/login', { replace: true }); clearUrlHash(); }
98-101: Fix grammar in mismatch message- toast.error("Passwords doesn't match. Try Again"); + toast.error("Passwords don't match. Try again.");
146-161: Improve UX and browser behavior: add autocomplete, minlength, and aria-labelsThese help password managers and accessibility, and align with your validation rules.
<InputField icon={Key} type="password" name="password-1" - className="mb-7" + className="mb-7" placeholder="New Password" + aria-label="New password" + autoComplete="new-password" + minLength={8} required /> <InputField icon={Key} type="password" name="password-2" - className="mb-7" + className="mb-7" placeholder="Reenter Password" + aria-label="Confirm new password" + autoComplete="new-password" + minLength={8} required />
135-135: Mount a single Toaster at the app rootConsider moving
<Toaster />to a top-level (e.g., App.tsx) to avoid multiple instances across pages and reduce DOM duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/App.tsx(3 hunks)frontend/src/components/pages/ForgotPasswrdPage.tsx(1 hunks)frontend/src/components/pages/ResetPasswordPage.tsx(1 hunks)frontend/src/components/pages/SignUpPage.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/components/pages/ForgotPasswrdPage.tsx
- frontend/src/App.tsx
- frontend/src/components/pages/SignUpPage.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
frontend/src/components/pages/ResetPasswordPage.tsx (2)
2-2: Import React types explicitly and avoidReact.namespace referencesMatches prior feedback: import the types and update the interface to prevent namespace issues in strict TS configs.
-import type { ReactNode, FormEvent } from "react"; +import type { ReactNode, FormEvent, InputHTMLAttributes, ElementType } from "react"; @@ -interface InputFieldProps extends React.InputHTMLAttributes<HTMLInputElement> { - icon: React.ElementType; +interface InputFieldProps extends InputHTMLAttributes<HTMLInputElement> { + icon: ElementType; }Also applies to: 17-19
35-45: Preserve caller-provided className in InputFieldCurrent
{...props}+ fixedclassNamedrops caller classes (e.g., Line 155, 163). Merge them.-const InputField = ({ icon: Icon, ...props }: InputFieldProps) => ( +const InputField = ({ icon: Icon, className, ...props }: InputFieldProps) => ( <div className="relative"> <div className="absolute inset-y-0 left-0 pl-3 flex items-center pointer-events-none"> <Icon className="h-5 w-5 text-gray-400" /> </div> <input - {...props} - className="block w-full pl-10 pr-3 py-2 border border-gray-800 rounded-lg bg-gray-900 text-white placeholder-gray-400 focus:outline-none focus:ring-2 focus:ring-green-500 focus:border-transparent" + {...props} + className={`block w-full pl-10 pr-3 py-2 border border-gray-800 rounded-lg bg-gray-900 text-white placeholder-gray-400 focus:outline-none focus:ring-2 focus:ring-green-500 focus:border-transparent ${className ?? ""}`} /> </div> );
🧹 Nitpick comments (3)
frontend/src/components/pages/ResetPasswordPage.tsx (3)
98-101: Use strict comparison and fix grammarAvoid type-coercing
!=and tweak the message.- if (password != repassword) { - toast.error("Passwords doesn't match. Try Again"); + if (password !== repassword) { + toast.error("Passwords don't match. Try again."); return; }
118-121: Refine “special character” check (spaces currently count as special)
/[^A-Za-z0-9]/matches spaces and other whitespace. If spaces shouldn’t satisfy the rule, use a stricter check.- if (!/[^A-Za-z0-9]/.test(password)) { - toast.error("Password must contain at least one special character."); + if (!/[^\w\s]/.test(password)) { + toast.error("Password must contain at least one non-alphanumeric symbol (excluding spaces)."); return; }If you prefer an allowlist, we can switch to an explicit symbol set (e.g.,
!@#$...). Confirm desired policy and I’ll adjust.
151-166: Improve UX and browser integration for password fieldsAdd
autoComplete="new-password"andminLengthhints; consider ARIA labels.<InputField icon={Key} type="password" name="password-1" className="mb-7" placeholder="New Password" + autoComplete="new-password" + minLength={8} + aria-label="New password" required /> <InputField icon={Key} type="password" name="password-2" className="mb-7" placeholder="Reenter Password" + autoComplete="new-password" + minLength={8} + aria-label="Confirm new password" required />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/pages/ResetPasswordPage.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/components/pages/ResetPasswordPage.tsx (2)
frontend/src/lib/supabaseClient.tsx (1)
supabase(12-12)backend/app/core/config/settings.py (1)
Settings(8-51)
🪛 Biome (2.1.2)
frontend/src/components/pages/ResetPasswordPage.tsx
[error] 202-202: expected } but instead the file ends
the file ends here
(parse)
🔇 Additional comments (1)
frontend/src/components/pages/ResetPasswordPage.tsx (1)
122-135: Good: robust loading state and success feedback aroundupdateUserThis addresses prior feedback: loading is reset via
finally, errors are handled, and success shows a toast before navigating.
smokeyScraper
left a comment
smokeyScraper
left a comment
There was a problem hiding this comment.
A few things to be taken care of -
- Even if the user has never registered, the forgot password option still sends a mail to them. However it does nothing and just redirects to login.
- Could you please add on a logout button too in the profile section. The only way to logout currently is by clearing browser cache.
- Auth state not subscribed globally (can break post-reset flow). Only an initial
getSession()runs; the app doesn't react to token changes (confirm email, password reset, sign-out in other tabs). Something likeonAuthStateChangeshould be implemented. - Multiple toasters being rendered throughout. There's a global toaster in App. Please keep a single global toaster.
- supabaseClient.tsx contains no JSX; rename to supabaseClient.ts.
- .env example is missing VITE_BASE_URL.
- reset-password seems to be not implement. It can probably be added in the profile section.
Could you please resolve 1, 3, 4, 5, 6.
Rest for 2 and 7 separate issue can be raised.
Thanks a lot for you contribution. Rest PR seems fine to me.
| <p className="text-center text-gray-400 text-sm"> | ||
| <button | ||
| type="button" | ||
| onClick={() => navigate('/signup')} |
There was a problem hiding this comment.
this should re-direct to sign in. It says "Back to SignIn" but redirect to SignUp
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/components/pages/ForgotPasswrdPage.tsx (1)
115-121: Back-to-sign-in route is now correct — thanks for fixingThis resolves the earlier review comment about the button routing to
/signup.Also applies to: 133-138
🧹 Nitpick comments (13)
frontend/src/lib/supabaseClient.ts (3)
4-9: Fail-fast throw is fine; add a guard against leaking service_role keys and consider a softer UX in prod.
- Add a lightweight runtime check to block accidental use of a service_role key in the browser.
- Optional: in production builds, prefer rendering a friendly error screen instead of throwing at module load (which white-screens the app).
Apply:
const supabaseUrl = import.meta.env.VITE_SUPABASE_URL || ""; const supabaseAnonKey = import.meta.env.VITE_SUPABASE_KEY || ""; -if (!supabaseUrl || !supabaseAnonKey) { +// Prevent accidental use of a service_role key in the browser +if (typeof window !== 'undefined' && supabaseAnonKey) { + try { + const payload = JSON.parse(atob((supabaseAnonKey.split('.')[1] || ''))); + if (payload?.role === 'service_role') { + throw new Error('Refusing to initialize Supabase with a service_role key in the browser. Use the anon public key instead.'); + } + } catch { + // noop: non-JWT or decoding failure; continue + } +} + +if (!supabaseUrl || !supabaseAnonKey) { throw new Error('Missing Supabase configuration. Please set VITE_SUPABASE_URL and VITE_SUPABASE_KEY environment variables.'); }
12-12: Explicit auth options for SPA stability.Defaults are OK, but being explicit helps future readers and avoids regressions.
-export const supabase = createClient(supabaseUrl, supabaseAnonKey) +export const supabase = createClient(supabaseUrl, supabaseAnonKey, { + auth: { persistSession: true, autoRefreshToken: true, detectSessionInUrl: true }, +})
4-6: Env var usage verified
- No occurrences of
service_roleor related keys found in the codebase.- Recommend updating documentation to clarify that
VITE_SUPABASE_KEYmust be the anon public key, never a service_role key.frontend/src/components/pages/ProfilePage.tsx (2)
182-194: Minor: make the button type explicit and add an accessible label.Prevents unintended form submissions if this moves inside a form later, and improves a11y.
- <motion.button + <motion.button + type="button" whileHover={{ scale: 1.05 }} whileTap={{ scale: 0.95 }} onClick={() => { onSignOut(); }} className="px-4 py-2 bg-red-500 hover:bg-red-600 rounded-lg transition-colors flex items-center" + aria-label="Sign out" > <DoorClosed size={18} className="mr-2" /> Sign Out </motion.button>
4-4: Icon choice nit.DoorClosed works, but LogOut better matches the intent. Optional swap.
frontend/src/App.tsx (3)
29-36: Clarify session-check error message and set landing page post-restore.
- Current toast reads “User Login Failed” on a session lookup error, which is misleading.
- When a session exists, optionally default users to the dashboard to match the linked issue’s objective.
- supabase.auth.getSession().then(({ data, error }) => { - if (error) { - toast.error('User Login Failed'); + supabase.auth.getSession().then(({ data, error }) => { + if (error) { + toast.error('Failed to restore session'); console.error('Error checking session:', error); return; } - setIsAuthenticated(!!data.session); + const hasSession = !!data.session; + setIsAuthenticated(hasSession); + if (hasSession) setActivePage('dashboard'); });
38-66: Tighten subscription destructuring and navigate to dashboard on sign-in; also avoid double toasts.
- Destructure subscription directly and clean up with subscription.unsubscribe().
- Set activePage to 'dashboard' on SIGNED_IN.
- Since SIGNED_OUT already toasts here, drop the success toast from handleLogout to prevent duplicates.
- const { data: subscription } = supabase.auth.onAuthStateChange( - (event, session) => { + const { data: { subscription } } = supabase.auth.onAuthStateChange( + (event, session) => { console.log("Auth event:", event, session); switch(event){ case "SIGNED_IN": setIsAuthenticated(true); + setActivePage('dashboard'); toast.success("Signed in!"); break; case "SIGNED_OUT": setIsAuthenticated(false); setActivePage("landing"); setRepoData(null); toast.success("Signed out!"); break; @@ - return () => { - subscription.subscription.unsubscribe(); - }; + return () => { + subscription.unsubscribe(); + };Also applies to: 67-69
77-85: Avoid duplicate logout state updates and toasts; rely on onAuthStateChange.Let the SIGNED_OUT handler manage UI state and success toast.
const handleLogout = async () => { const { error } = await supabase.auth.signOut(); if (error) { toast.error('Logout failed'); console.error('Error during logout:', error); - return; + return; } - toast.success('Signed out!'); - setIsAuthenticated(false); - setActivePage('landing'); - setRepoData(null); };frontend/src/components/pages/ForgotPasswrdPage.tsx (5)
1-1: Use explicit React type imports and honor caller className in InputField
- Import React types directly to avoid “Cannot find namespace 'React'.” in some TS configs.
- Merge
classNamefrom props; currently it’s ignored because the component overwrites it.-import { useState, ReactNode, FormEvent } from "react"; +import { useState } from "react"; +import type { ReactNode, FormEvent, InputHTMLAttributes, ElementType } from "react"; @@ -interface InputFieldProps extends React.InputHTMLAttributes<HTMLInputElement> { - icon: React.ElementType; +interface InputFieldProps extends InputHTMLAttributes<HTMLInputElement> { + icon: ElementType; } @@ -const InputField = ({ icon: Icon, ...props }: InputFieldProps) => ( +const InputField = ({ icon: Icon, className, ...props }: InputFieldProps) => ( <div className="relative"> <div className="absolute inset-y-0 left-0 pl-3 flex items-center pointer-events-none"> <Icon className="h-5 w-5 text-gray-400" /> </div> <input - {...props} - className="block w-full pl-10 pr-3 py-2 border border-gray-800 rounded-lg bg-gray-900 text-white placeholder-gray-400 focus:outline-none focus:ring-2 focus:ring-green-500 focus:border-transparent" + {...props} + className={`block w-full pl-10 pr-3 py-2 border border-gray-800 rounded-lg bg-gray-900 text-white placeholder-gray-400 focus:outline-none focus:ring-2 focus:ring-green-500 focus:border-transparent ${className ?? ''}`} /> </div> );Also applies to: 14-20, 36-46
85-92: Minor a11y and UX: add autocomplete and aria-label to email fieldImproves accessibility and browser autofill.
<InputField icon={Mail} type="email" name="email" - className="mb-7" + className="mb-7" placeholder="Email address" + aria-label="Email address" + autoComplete="email" required />
49-49: Fix spelling: rename component to ForgotPasswordPageKeeps names consistent and easier to grep.
-export default function ForgotPasswrdPage() { +export default function ForgotPasswordPage() {If routing imports rely on the default export only, this change is non-breaking. If the filename is also misspelled, consider renaming the file in a follow-up.
134-134: Remove redundant state update before navigation
navigate('/login')will unmount this component;setMailPage(false)is unnecessary.- onClick={() => { setMailPage(false); navigate('/login') }} + onClick={() => { navigate('/login') }}
4-4: Import spacing nitTiny formatting fix keeps imports consistent with the rest of the codebase.
-import { toast} from "react-hot-toast"; +import { toast } from "react-hot-toast";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
frontend/src/App.tsx(4 hunks)frontend/src/components/pages/ForgotPasswrdPage.tsx(1 hunks)frontend/src/components/pages/ProfilePage.tsx(2 hunks)frontend/src/components/pages/ResetPasswordPage.tsx(1 hunks)frontend/src/components/pages/SignUpPage.tsx(1 hunks)frontend/src/lib/supabaseClient.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/components/pages/ResetPasswordPage.tsx
- frontend/src/components/pages/SignUpPage.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/App.tsx (4)
frontend/src/lib/supabaseClient.ts (1)
supabase(12-12)frontend/src/components/pages/ForgotPasswrdPage.tsx (1)
ForgotPasswrdPage(49-144)frontend/src/components/pages/ResetPasswordPage.tsx (1)
ResetPasswordPage(50-201)frontend/src/components/pages/SignUpPage.tsx (1)
SignUpPage(51-210)
frontend/src/components/pages/ForgotPasswrdPage.tsx (1)
frontend/src/lib/supabaseClient.ts (1)
supabase(12-12)
🔇 Additional comments (6)
frontend/src/components/pages/ProfilePage.tsx (1)
6-6: Prop API change looks good.App.tsx wires onSignOut correctly. No issues.
frontend/src/App.tsx (5)
4-4: Toaster import/use LGTM.Placement at top-right is fine.
16-19: Auth routes/components imports are fine.Nothing to flag.
109-109: ProfilePage wiring LGTM.Prop passed correctly.
130-153: Route gating looks correct.
- forgot-password and signup are public when unauthenticated.
- reset-password accessible as intended.
73-76: Potential duplication with onAuthStateChange.If LoginPage already signs in via Supabase, onAuthStateChange will set isAuthenticated; consider removing handleLogin or ensure it’s only called after a successful sign-in to avoid flicker.
smokeyScraper
left a comment
smokeyScraper
left a comment
There was a problem hiding this comment.
hey @PinJinx, can you please address the recent coderrabit issue? If that seems something relevant please do the needful.
Also please can you please change the environment variable name to
SUPABASE_URL from VITE_SUPABASE_URL and SUPABASE_KEY from VITE_SUPABASE_KEY as both of these are same.
Also if possible try to restructure the commits a bit,
In order to have a clean commit history, please undo your last commits and make changes within that, then make a commit with a force push. This keeps the commit history clean. I'm preferring "'n' commits for edits" + "1 commit for coderabbit fix" (Easiest is to use editor's (VS Code/Cursor) GitHub section for this)
Migrates Discord bot commands from legacy prefix-based to modern slash commands using discord.py app_commands. Refactors DevRelCommands cog to use slash commands, improves token cleanup task management, and updates verification flows for ephemeral responses. Discord bot now loads cogs dynamically during async startup, and the Discord dependency is added to pyproject.toml.
…contributor recommendation tool
…rize and embed it
…thub authorization
feat: Added env example feat: Added Forgot Password Page
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
frontend/src/components/pages/ResetPasswordPage.tsx (2)
2-2: Import React types explicitly; avoid React namespace usagePrevents “Cannot find namespace 'React'” in stricter TS configs.
-import type { ReactNode, FormEvent } from "react"; +import type { ReactNode, FormEvent, InputHTMLAttributes, ElementType } from "react"; @@ -interface InputFieldProps extends React.InputHTMLAttributes<HTMLInputElement> { - icon: React.ElementType; +interface InputFieldProps extends InputHTMLAttributes<HTMLInputElement> { + icon: ElementType; }Also applies to: 17-19
35-45: Preserve caller-provided className in InputFieldCurrent order overrides consumer classes (e.g., mb-7).
-const InputField = ({ icon: Icon, ...props }: InputFieldProps) => ( +const InputField = ({ icon: Icon, className, ...props }: InputFieldProps) => ( <div className="relative"> @@ - <input - {...props} - className="block w-full pl-10 pr-3 py-2 border border-gray-800 rounded-lg bg-gray-900 text-white placeholder-gray-400 focus:outline-none focus:ring-2 focus:ring-green-500 focus:border-transparent" - /> + <input + {...props} + className={`block w-full pl-10 pr-3 py-2 border border-gray-800 rounded-lg bg-gray-900 text-white placeholder-gray-400 focus:outline-none focus:ring-2 focus:ring-green-500 focus:border-transparent ${className ?? ""}`} + /> </div> );
🧹 Nitpick comments (26)
frontend/src/components/pages/ResetPasswordPage.tsx (5)
96-97: Avoid unsafe cast from FormData.get; coerce safely to stringPrevents a runtime error if a field is unexpectedly missing.
- const password = formData.get('password-1') as string; - const repassword = formData.get('password-2') as string; + const password = formData.get('password-1')?.toString() ?? ""; + const repassword = formData.get('password-2')?.toString() ?? "";
98-101: Use strict equality and fix grammar in toastTiny correctness/readability win.
- if (password != repassword) { - toast.error("Passwords doesn't match. Try Again"); + if (password !== repassword) { + toast.error("Passwords don't match. Try again.");
56-67: Constrain reset flow to recovery links onlyCheck the
typeparam to ensure we only accept recovery tokens.- const params = new URLSearchParams(window.location.hash.slice(1)); + const params = new URLSearchParams(window.location.hash.slice(1)); @@ - const accessToken = params.get('access_token'); - const refreshToken = params.get('refresh_token'); + const accessToken = params.get('access_token'); + const refreshToken = params.get('refresh_token'); + const linkType = params.get('type'); @@ - if (accessToken && refreshToken) { + if (accessToken && refreshToken && linkType === 'recovery') {
151-165: Improve UX/accessibility: autocomplete, minLength, aria-labelsHelps browsers/autofill and screen readers.
<InputField icon={Key} type="password" name="password-1" className="mb-7" placeholder="New Password" + autoComplete="new-password" + minLength={8} + aria-label="New password" required /> <InputField icon={Key} type="password" name="password-2" className="mb-7" placeholder="Reenter Password" + autoComplete="new-password" + minLength={8} + aria-label="Reenter new password" required />
166-172: Expose loading state to assistive techAdvertise progress on the submit button.
<motion.button whileHover={{ scale: 1.02 }} whileTap={{ scale: 0.98 }} type="submit" disabled={isLoading} + aria-busy={isLoading} className="w-full py-3 bg-green-500 hover:bg-green-600 text-white rounded-lg font-medium transition-colors flex items-center justify-center" >backend/app/agents/devrel/prompts/response_prompt.py (1)
33-40: Add one more constraint for deterministic lists.Consider specifying score formatting to keep outputs consistent.
SPECIAL FORMATTING FOR CONTRIBUTOR RECOMMENDATIONS: If the task result contains contributor recommendations: - Start with "Found X Contributors" - Show search query used and keywords -- For each contributor: "1. username (Score: X.XXX)" +- For each contributor: "1. username (Score: X.XXX)" — format scores to 3 decimals - Include their expertise/reason for recommendation - End with metadata about search and actionable guidancebackend/integrations/discord/bot.py (5)
39-43: Consider more specific exception handling for slash command sync.While catching the broad Exception helps prevent startup failures, consider catching more specific exceptions (e.g.,
discord.HTTPException,discord.Forbidden) to better understand and handle different failure scenarios.try: synced = await self.tree.sync() print(f"Synced {len(synced)} slash command(s)") -except Exception as e: +except discord.HTTPException as e: + print(f"Failed to sync slash commands due to HTTP error: {e}") +except discord.Forbidden as e: + print(f"Failed to sync slash commands due to permissions: {e}") +except Exception as e: print(f"Failed to sync slash commands: {e}")
91-94: Consider extracting priority mapping as a class constant.The priority mapping is a static configuration that could be defined once at the class level rather than recreated on each message.
Add this as a class constant:
PRIORITY_MAP = { "high": QueuePriority.HIGH, "medium": QueuePriority.MEDIUM, "low": QueuePriority.LOW }Then use it in the method:
-priority_map = {"high": QueuePriority.HIGH, - "medium": QueuePriority.MEDIUM, - "low": QueuePriority.LOW -} -priority = priority_map.get(triage_result.get("priority"), QueuePriority.MEDIUM) +priority = self.PRIORITY_MAP.get(triage_result.get("priority"), QueuePriority.MEDIUM)
105-106: Uselogger.exceptionfor better error tracking.When catching exceptions in an except block, use
logger.exceptioninstead oflogger.errorto automatically include the traceback.except Exception as e: - logger.error(f"Error handling DevRel message: {str(e)}") + logger.exception("Error handling DevRel message")
125-127: Uselogger.exceptionfor better error tracking.Similar to the previous comment, use
logger.exceptionfor automatic traceback inclusion.except Exception as e: - logger.error(f"Failed to create thread: {e}") + logger.exception("Failed to create thread")
141-142: Uselogger.exceptionfor better error tracking.except Exception as e: - logger.error(f"Error handling agent response: {str(e)}") + logger.exception("Error handling agent response")backend/integrations/discord/cogs.py (3)
36-37: Consider more specific exception handling.While catching broad exceptions prevents task failures from crashing the bot, consider catching more specific exceptions or at least logging the full traceback.
except Exception as e: - logger.error(f"Error during token cleanup: {e}") + logger.exception("Error during token cleanup")
99-101: Uselogger.exceptionfor better error tracking.except Exception as e: - logger.error(f"Error checking verification status: {e}") + logger.exception("Error checking verification status") await interaction.response.send_message("❌ Error checking verification status.", ephemeral=True)
172-173: Uselogger.exceptionfor better error tracking.except Exception as e: - logger.error(f"Error in /verify_github: {e}") + logger.exception("Error in /verify_github command")backend/main.py (5)
47-53: Decide on fail-fast vs. degrade if the Discord cog fails to loadYou log and continue when the extension fails. If commands are critical, fail startup; if optional, at least surface a health signal.
Apply either approach:
Option A (fail-fast):
- try: - await self.discord_bot.load_extension("integrations.discord.cogs") - except (ImportError, commands.ExtensionError) as e: - logger.error("Failed to load Discord cog extension: %s", e) + try: + await self.discord_bot.load_extension("integrations.discord.cogs") + except (ImportError, commands.ExtensionError): + logger.exception("Failed to load Discord cog extension") + raiseOption B (degrade, but expose status):
- Track a boolean self.discord_ext_loaded and expose it via a health endpoint/metric so operators know commands are unavailable.
59-61: Prefer logger.exception in except blocksIt automatically includes the traceback and satisfies TRY400.
- logger.error(f"Error during background task startup: {e}", exc_info=True) + logger.exception("Error during background task startup")
63-71: Treat Weaviate not-ready as a startup failure (or log clearly)Currently you only log when ready; a False readiness silently proceeds.
- if await client.is_ready(): - logger.info("Weaviate connection successful and ready") + if await client.is_ready(): + logger.info("Weaviate connection successful and ready") + else: + logger.error("Weaviate connected but not ready") + raise RuntimeError("Weaviate not ready at startup")If Weaviate is optional, at least log a warning and set a health flag.
77-81: Shutdown path is good; align logging to exception helperSmall consistency tweak.
- except Exception as e: - logger.error(f"Error closing Discord bot: {e}", exc_info=True) + except Exception: + logger.exception("Error closing Discord bot")Do the same for the queue manager block.
125-130: Uvicorn target is fine for script runs; ensure env validation covers ASGI runsEnv checks run only in main. If deployed via
uvicorn backend.main:api, missing secrets won’t be validated. Consider validating critical settings instart_background_tasks.backend/app/agents/devrel/github/github_toolkit.py (1)
9-9: Tool import enabled; consider lazy import to reduce cold-start and avoid cyclesDelay heavy/optional deps until actually used.
- from .tools.contributor_recommendation import handle_contributor_recommendation + # Lazy import in execute() to reduce import-time couplingAnd inside the branch:
from .tools.contributor_recommendation import handle_contributor_recommendation # local importbackend/app/agents/devrel/github/prompts/contributor_recommendation/issue_summarization.py (1)
14-20: Delimit injected issue content to reduce prompt-injection risk and improve LLM parsingWrap
{issue_content}in explicit code fences so the model treats it as verbatim context. This also helps when the issue body contains JSON-like braces.-**GitHub Issue Content:** ---- -{issue_content} ---- +**GitHub Issue Content (verbatim):** +--- +```markdown +{issue_content} +``` +--- **Optimized Technical Summary for Contributor Search:**backend/app/agents/devrel/github/tools/contributor_recommendation.py (4)
36-43: Bracket GitHub issue content when composing the alignment promptUse explicit sentinels to segregate raw issue text from the user’s query. This improves robustness of downstream JSON alignment.
- if url_match: - issue_content = await self._fetch_github_issue_content(url_match.group(0)) - full_query = f"{query}\n\nIssue content: {issue_content}" + if url_match: + issue_content = await self._fetch_github_issue_content(url_match.group(0)) + full_query = ( + f"{query}\n\n[BEGIN_GITHUB_ISSUE]\n{issue_content}\n[END_GITHUB_ISSUE]" + ) else: full_query = query
77-79: Log with traceback and re-raise without shadowingUse
logger.exceptionto capture the stack trace and prefer bareraiseto preserve the original context.- except Exception as e: - logger.error(f"GitHub issue fetching failed: {e}") - raise + except Exception: + logger.exception("GitHub issue fetching failed") + raise
100-106: Make search weighting configurable via settingsHard-coded weights make tuning difficult. Consider moving these to config (e.g.,
settings.vector_weight,settings.bm25_weight) with sensible defaults.
168-170: Uselogger.exceptionfor unified error reportingThis captures the traceback without needing
exc_info=True, and avoids manual string conversion.- except Exception as e: - logger.error(f"Error in contributor recommendation: {str(e)}", exc_info=True) - return {"status": "error", "message": str(e)} + except Exception as e: + logger.exception("Error in contributor recommendation") + return {"status": "error", "message": str(e)}backend/app/services/github/issue_processor.py (1)
82-83: Preferlogger.exceptionand bareraiseKeeps full traceback and avoids re-wrapping the exception.
- except Exception as e: - logger.error(f"Error processing issue {self.owner}/{self.repo}#{self.issue_number}: {str(e)}") - raise e + except Exception: + logger.exception("Error processing issue %s/%s#%s", self.owner, self.repo, self.issue_number) + raise
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
frontend/package-lock.jsonis excluded by!**/package-lock.jsonpoetry.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
.vimrc(1 hunks)backend/app/agents/devrel/github/github_toolkit.py(2 hunks)backend/app/agents/devrel/github/prompts/contributor_recommendation/issue_summarization.py(1 hunks)backend/app/agents/devrel/github/prompts/contributor_recommendation/query_alignment.py(1 hunks)backend/app/agents/devrel/github/prompts/intent_analysis.py(2 hunks)backend/app/agents/devrel/github/tools/contributor_recommendation.py(1 hunks)backend/app/agents/devrel/prompts/response_prompt.py(1 hunks)backend/app/api/v1/auth.py(1 hunks)backend/app/services/github/issue_processor.py(1 hunks)backend/app/services/github/user/profiling.py(1 hunks)backend/integrations/discord/bot.py(4 hunks)backend/integrations/discord/cogs.py(7 hunks)backend/main.py(2 hunks)backend/requirements.txt(1 hunks)docs/INSTALL_GUIDE.md(1 hunks)frontend/.env example(1 hunks)frontend/package.json(1 hunks)frontend/src/App.tsx(4 hunks)frontend/src/components/pages/ForgotPasswrdPage.tsx(1 hunks)frontend/src/components/pages/LoginPage.tsx(4 hunks)frontend/src/components/pages/ProfilePage.tsx(2 hunks)frontend/src/components/pages/ResetPasswordPage.tsx(1 hunks)frontend/src/components/pages/SignUpPage.tsx(1 hunks)frontend/src/lib/supabaseClient.ts(1 hunks)pyproject.toml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- backend/app/api/v1/auth.py
- .vimrc
🚧 Files skipped from review as they are similar to previous changes (10)
- docs/INSTALL_GUIDE.md
- frontend/src/components/pages/LoginPage.tsx
- frontend/.env example
- frontend/src/components/pages/SignUpPage.tsx
- frontend/package.json
- frontend/src/lib/supabaseClient.ts
- frontend/src/components/pages/ForgotPasswrdPage.tsx
- frontend/src/App.tsx
- backend/requirements.txt
- frontend/src/components/pages/ProfilePage.tsx
🧰 Additional context used
🧬 Code graph analysis (7)
backend/app/agents/devrel/github/tools/contributor_recommendation.py (3)
backend/app/database/weaviate/operations.py (1)
search_contributors(366-379)backend/app/services/github/issue_processor.py (2)
GitHubIssueProcessor(14-83)fetch_issue_content(30-56)backend/app/services/embedding_service/service.py (3)
EmbeddingService(29-226)model(41-52)get_embedding(70-90)
backend/app/agents/devrel/github/github_toolkit.py (1)
backend/app/agents/devrel/github/tools/contributor_recommendation.py (1)
handle_contributor_recommendation(81-170)
backend/app/services/github/issue_processor.py (2)
backend/app/services/embedding_service/service.py (3)
EmbeddingService(29-226)model(41-52)get_embedding(70-90)backend/app/services/github/user/profiling.py (2)
GitHubUserProfiler(15-298)request(77-79)
backend/main.py (2)
backend/app/core/orchestration/queue_manager.py (1)
start(47-56)backend/app/database/weaviate/client.py (1)
get_weaviate_client(19-32)
frontend/src/components/pages/ResetPasswordPage.tsx (1)
frontend/src/lib/supabaseClient.ts (1)
supabase(12-12)
backend/integrations/discord/bot.py (2)
backend/app/classification/classification_router.py (1)
ClassificationRouter(10-59)backend/app/core/orchestration/queue_manager.py (2)
QueuePriority(12-15)enqueue(73-92)
backend/integrations/discord/cogs.py (3)
backend/integrations/discord/bot.py (1)
on_ready(35-43)backend/app/services/auth/management.py (1)
get_or_create_user_by_discord(10-41)backend/app/services/auth/verification.py (1)
create_verification_session(33-61)
🪛 Ruff (0.12.2)
backend/app/agents/devrel/github/tools/contributor_recommendation.py
52-52: Consider moving this statement to an else block
(TRY300)
75-75: Abstract raise to an inner function
(TRY301)
75-75: Avoid specifying long messages outside the exception class
(TRY003)
78-78: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
169-169: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backend/app/services/github/issue_processor.py
41-41: Avoid specifying long messages outside the exception class
(TRY003)
74-74: Abstract raise to an inner function
(TRY301)
74-74: Avoid specifying long messages outside the exception class
(TRY003)
80-80: Consider moving this statement to an else block
(TRY300)
82-82: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
82-82: Use explicit conversion flag
Replace with conversion flag
(RUF010)
83-83: Use raise without specifying exception name
Remove exception name
(TRY201)
backend/main.py
51-51: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
54-56: Store a reference to the return value of asyncio.create_task
(RUF006)
backend/integrations/discord/bot.py
42-42: Do not catch blind exception: Exception
(BLE001)
105-105: Do not catch blind exception: Exception
(BLE001)
106-106: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
106-106: Use explicit conversion flag
Replace with conversion flag
(RUF010)
125-125: Do not catch blind exception: Exception
(BLE001)
126-126: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
141-141: Do not catch blind exception: Exception
(BLE001)
142-142: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
142-142: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backend/integrations/discord/cogs.py
99-99: Do not catch blind exception: Exception
(BLE001)
100-100: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
172-172: Do not catch blind exception: Exception
(BLE001)
173-173: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🔇 Additional comments (15)
frontend/src/components/pages/ResetPasswordPage.tsx (1)
67-84: Session setup + hash clearing look goodNice use of try/catch/finally with
replaceStateto drop tokens.backend/app/agents/devrel/prompts/response_prompt.py (1)
29-29: LGTM: simpler emoji guidance.Dropping the hardcoded emoji list reduces prompt brittleness across channels.
backend/app/agents/devrel/github/prompts/contributor_recommendation/query_alignment.py (1)
1-37: LGTM! Well-structured prompt for query alignment.The prompt template effectively guides the transformation of user requests into technical search queries, with clear examples and strict formatting rules to ensure consistent JSON output.
backend/integrations/discord/bot.py (3)
21-21: Good migration to slash commands.Setting
command_prefix=Noneproperly indicates this bot uses slash commands exclusively, aligning with modern Discord bot best practices.
46-51: Smart filtering of slash command interactions.Good use of
interaction_metadatato properly distinguish and skip slash command interactions, preventing double-processing of commands.
137-138: Efficient message chunking implementation.Good implementation of Discord's 2000-character message limit handling with a simple loop.
backend/integrations/discord/cogs.py (5)
20-24: Good implementation of task lifecycle management.The
on_readylistener properly checks if the cleanup task is already running before starting it, preventing duplicate task instances.
44-55: Clean implementation of the reset command.The slash command properly handles user context, enqueues the cleanup task with high priority, and provides user feedback via ephemeral message.
57-74: Well-structured help command with clear formatting.The help embed provides clear documentation of available commands with proper slash command syntax.
106-106: Good use of deferred response for long-running operations.Deferring the response with
ephemeral=Trueis the correct approach for operations that may take longer than Discord's 3-second timeout.
181-183: Proper cog setup function implementation.The
setupfunction correctly implements Discord.py's extension loading pattern, properly registering the cog with the bot.backend/main.py (1)
15-17: Import for ExtensionError looks correctUsing discord.ext.commands.ExtensionError is valid for extension load failures. No action needed.
backend/app/agents/devrel/github/github_toolkit.py (1)
104-106: Route looks correct; ensure consistent result shape and failure handling
handle_contributor_recommendationreturns a dict with status/message; you already attach intent/type. LGTM.Optionally add a guard to normalize non-dict returns:
if not isinstance(result, dict): result = {"status": "error", "message": "Invalid tool response", "raw": str(result)}backend/app/agents/devrel/github/prompts/intent_analysis.py (2)
5-5: Clearer function description for contributor_recommendationUpdated scope is crisp and will improve routing. LGTM.
15-21: Good, concrete examples improve LLM accuracyThe examples cover PRs, tech domains, and issue URLs, which should reduce misclassification.
| try: | ||
| import json | ||
| print(response) | ||
| result = json.loads(response.content.strip()) | ||
| logger.info(f"Query aligned: '{result.get('aligned_query')}' with keywords: {result.get('keywords')}") | ||
| return result | ||
| except json.JSONDecodeError: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace print with structured logging and tolerate fenced JSON from the LLM
Printing backend objects is noisy and can leak data in some environments. Also, LLMs often wrap JSON in triple backticks; parse that defensively before json.loads.
try:
- import json
- print(response)
- result = json.loads(response.content.strip())
- logger.info(f"Query aligned: '{result.get('aligned_query')}' with keywords: {result.get('keywords')}")
+ import json
+ raw = response.content or ""
+ logger.debug("Alignment raw response (first 500 chars): %s", raw[:500])
+ match = re.search(r'```(?:json)?\s*(\{.*?\})\s*```', raw, re.S)
+ payload = match.group(1) if match else raw.strip()
+ result = json.loads(payload)
+ logger.info("Query aligned: '%s' with keywords: %s", result.get("aligned_query"), result.get("keywords"))
return resultCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.12.2)
52-52: Consider moving this statement to an else block
(TRY300)
🤖 Prompt for AI Agents
In backend/app/agents/devrel/github/tools/contributor_recommendation.py around
lines 47 to 53, replace the raw print(response) with structured logger calls and
defensively handle LLM-wrapped JSON: strip surrounding triple backticks or other
fencing before calling json.loads by using a regex to extract the innermost JSON
object (e.g., find a { ... } block) or fallback to the stripped raw content,
then call json.loads on that payload; use logger.info with structured arguments
instead of f-strings/print to log the aligned_query and keywords, and ensure the
JSONDecodeError except branch logs the error and the raw payload for debugging.
| issue_data = await profiler.request(issue_url) | ||
| if not issue_data: | ||
| raise ValueError("Failed to fetch issue data.") | ||
|
|
||
| content_parts = [ | ||
| f"Title: {issue_data['title']}", | ||
| f"Body: {issue_data['body']}", | ||
| ] | ||
|
|
||
| comments_data = await profiler.request(comments_url) | ||
| if comments_data: | ||
| comment_texts = [ | ||
| f"Comment by {c['user']['login']}: {c['body']}" | ||
| for c in comments_data if c.get('body') | ||
| ] | ||
| content_parts.extend(comment_texts) | ||
|
|
||
| return "\n\n---\n\n".join(content_parts) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Defensive field access and length caps to keep prompts within model limits
GitHub payloads can have missing fields and very long bodies. Use .get(...) with defaults and cap sizes; also limit the number/size of comments.
- content_parts = [
- f"Title: {issue_data['title']}",
- f"Body: {issue_data['body']}",
- ]
-
- comments_data = await profiler.request(comments_url)
- if comments_data:
- comment_texts = [
- f"Comment by {c['user']['login']}: {c['body']}"
- for c in comments_data if c.get('body')
- ]
- content_parts.extend(comment_texts)
-
- return "\n\n---\n\n".join(content_parts)
+ # Be defensive about missing fields and cap lengths to contain token usage
+ title = (issue_data.get("title") or "")[:300]
+ body = (issue_data.get("body") or "")[:4000]
+ content_parts = [f"Title: {title}", f"Body: {body}"]
+
+ comments_data = await profiler.request(comments_url)
+ if comments_data:
+ comment_texts = []
+ for c in comments_data[:20]:
+ text = (c.get("body") or "").strip()
+ if not text:
+ continue
+ login = c.get("user", {}).get("login", "unknown")
+ comment_texts.append(f"Comment by {login}: {text[:1000]}")
+ content_parts.extend(comment_texts)
+
+ return "\n\n---\n\n".join(content_parts)📝 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.
| issue_data = await profiler.request(issue_url) | |
| if not issue_data: | |
| raise ValueError("Failed to fetch issue data.") | |
| content_parts = [ | |
| f"Title: {issue_data['title']}", | |
| f"Body: {issue_data['body']}", | |
| ] | |
| comments_data = await profiler.request(comments_url) | |
| if comments_data: | |
| comment_texts = [ | |
| f"Comment by {c['user']['login']}: {c['body']}" | |
| for c in comments_data if c.get('body') | |
| ] | |
| content_parts.extend(comment_texts) | |
| return "\n\n---\n\n".join(content_parts) | |
| # Be defensive about missing fields and cap lengths to contain token usage | |
| title = (issue_data.get("title") or "")[:300] | |
| body = (issue_data.get("body") or "")[:4000] | |
| content_parts = [f"Title: {title}", f"Body: {body}"] | |
| comments_data = await profiler.request(comments_url) | |
| if comments_data: | |
| comment_texts = [] | |
| for c in comments_data[:20]: | |
| text = (c.get("body") or "").strip() | |
| if not text: | |
| continue | |
| login = c.get("user", {}).get("login", "unknown") | |
| comment_texts.append(f"Comment by {login}: {text[:1000]}") | |
| content_parts.extend(comment_texts) | |
| return "\n\n---\n\n".join(content_parts) |
🧰 Tools
🪛 Ruff (0.12.2)
41-41: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In backend/app/services/github/issue_processor.py around lines 39 to 56, the
code directly indexes fields from the GitHub payload and can produce huge
prompts; change to use .get(...) with safe defaults for title, body, and comment
fields, truncate title/body/comment strings to a reasonable length (e.g.
500-2000 chars as appropriate), and limit the number of comments included (e.g.
first N comments) and/or total combined size to ensure prompt stays within model
limits; build content_parts using the safe, truncated values and skip empty
entries.
| async def request(self, url: str, params: Dict | None = None) -> Optional[Dict]: | ||
| """Public, stable wrapper around the internal HTTP helper.""" | ||
| return await self._make_request(url, params) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
PEP 604 “|” union breaks our Python 3.9 support.
requires-python is >=3.9; the X | Y syntax is only valid on Python 3.10+. Use Optional[Dict] instead. (peps.python.org, docs.python.org)
- async def request(self, url: str, params: Dict | None = None) -> Optional[Dict]:
+ async def request(self, url: str, params: Optional[Dict] = None) -> Optional[Dict]:🏁 Script executed:
#!/bin/bash
# Find other occurrences of PEP 604 unions that would break on 3.9
rg -nP --type=py -C1 '\bdef\b[^\n]*\([^)]*\|[^)]*\)'
rg -nP --type=py -C1 '->[^:\n]*\|[^:\n]*:'
rg -nP --type=py -C1 ':\s*[^#\n]*\|[^#\n]*'Length of output: 2372
PEP 604 union syntax breaks Python 3.9 support
requires-python >= 3.9; the X | Y union syntax is only valid on Python 3.10+. Replace all PEP 604 unions with Optional[...] (or Union[..., None]).
backend/app/services/github/user/profiling.py line 77:
- async def request(self, url: str, params: Dict | None = None) -> Optional[Dict]:
+ async def request(self, url: str, params: Optional[Dict] = None) -> Optional[Dict]:backend/app/core/events/base.py line 11: change
channel_id: str | None = Noneto
channel_id: Optional[str] = NoneEnsure from typing import Optional is imported in each file.
🤖 Prompt for AI Agents
In backend/app/services/github/user/profiling.py around lines 77-79 and
backend/app/core/events/base.py around line 11, replace PEP 604 union syntax (X
| None) with typing.Optional (e.g., Optional[X]) to retain Python 3.9
compatibility, and ensure each file imports Optional from typing; update the
function signature and variable annotations accordingly and remove the
pipe-style unions throughout these files.
| asyncio.create_task( | ||
| self.discord_bot.start(settings.discord_bot_token) | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Keep a reference to the created task and observe exceptions
Discarding the task can hide startup errors and makes shutdown harder.
- asyncio.create_task(
- self.discord_bot.start(settings.discord_bot_token)
- )
+ self._bot_task = asyncio.create_task(
+ self.discord_bot.start(settings.discord_bot_token)
+ )
+ self._bot_task.add_done_callback(
+ lambda t: logger.exception("Discord bot task errored", exc_info=t.exception())
+ if t.exception() else None
+ )Outside this hunk, add initialization and shutdown handling:
# __init__
self._bot_task = None# stop_background_tasks (before/after closing the bot)
if getattr(self, "_bot_task", None) and not self._bot_task.done():
self._bot_task.cancel()
try:
await self._bot_task
except asyncio.CancelledError:
pass🧰 Tools
🪛 Ruff (0.12.2)
54-56: Store a reference to the return value of asyncio.create_task
(RUF006)
🤖 Prompt for AI Agents
In backend/main.py around lines 54–56, the created discord bot task is discarded
which can hide startup exceptions and complicate shutdown; initialize
self._bot_task = None in __init__, assign the created task to self._bot_task
instead of discarding it, and in the shutdown/stop_background_tasks path check
if self._bot_task exists and is not done, call cancel() on it and await it
inside a try/except that ignores asyncio.CancelledError to ensure proper
cancellation and surfacing of other exceptions.
| "aio-pika (>=9.5.5,<10.0.0)", | ||
| "uvicorn (>=0.35.0,<0.36.0)", | ||
| "ddgs (>=9.0.2,<10.0.0)", | ||
| "discord-py (>=2.5.2,<3.0.0)", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Do not add discord-py alongside py-cord; pick one Discord library.
- py-cord (line 11) and discord.py share the same import namespace (import discord) and are intentionally incompatible; installing both leads to non-deterministic imports and runtime breakage. Pycord’s own docs instruct uninstalling discord.py before installing py-cord. Also, the official package name is discord.py, not discord-py. (guide.pycord.dev, pypi.org)
Apply one of the following:
Option A (keep Pycord; minimal change):
- "discord-py (>=2.5.2,<3.0.0)",Option B (migrate to official discord.py; remove Pycord too):
- "py-cord (>=2.6.1,<3.0.0)",
+ "discord.py (>=2.6.0,<3.0.0)",Then delete line 28 as in Option A.
Run to confirm there’s no mixed usage or stale pins:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show discord deps declared
rg -nP 'discord\.py|py-cord|discord-py' pyproject.toml
# Find imports to verify single-namespace usage
rg -nP --type=py -C2 '^\s*(from\s+discord\b|import\s+discord\b)'
# Spot other requirement files that may still pin discord libs
fd -a -E node_modules -E .venv -E venv -E dist 'requirements*.txt' \
| xargs -r -I{} rg -n 'discord\.py|py-cord|discord-py' {}Length of output: 1263
Remove conflicting Discord libraries
- Only install one Discord library—Pycord (
py-cord) or the officialdiscord.py—since both share the sameimport discordnamespace and conflict.
• Option A (keep Pycord): delete the"discord-py (>=2.5.2,<3.0.0)"entry (pyproject.toml line 28).
• Option B (migrate to discord.py): in pyproject.toml replace the"py-cord (>=2.6.1,<3.0.0)"entry (line 11) with"discord.py (>=2.6.0,<3.0.0)"and remove the discord-py line. - Update any requirements.txt (e.g. the pin
py-cord==2.6.1at line 156) to install only your chosen library.
🤖 Prompt for AI Agents
In pyproject.toml around line 28 the project installs two conflicting Discord
libraries (py-cord and discord-py) that share the same import namespace; remove
the conflict by choosing one option: Option A (keep Pycord) — delete the
"discord-py (>=2.5.2,<3.0.0)" entry at line 28 and ensure any other references
use py-cord; Option B (migrate to discord.py) — replace the "py-cord
(>=2.6.1,<3.0.0)" entry with "discord.py (>=2.6.0,<3.0.0)" and remove the
discord-py line; then update requirements.txt (remove or change the pinned
py-cord==2.6.1 at line 156) so only the chosen library is installed.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
frontend/src/App.tsx (1)
75-76: Nice: logout now calls Supabase signOut.This addresses the prior review about terminating the session server-side.
🧹 Nitpick comments (3)
frontend/src/App.tsx (3)
24-24: Avoidanyfor repoData.Consider introducing a
RepoDatatype and passing it through props to improve safety.
29-36: Minor copy tweak for the error toast.“User Login Failed” on a session fetch error can be misleading; suggest clarifying the message.
- toast.error('User Login Failed'); + toast.error('Session check failed');
47-51: Deduplicate “Signed out!” toast.Both the SIGNED_OUT handler and
handleLogoutshow a success toast, causing double notifications. Keep one (prefer the auth listener).const handleLogout = async () => { const { error } = await supabase.auth.signOut(); if (error) { toast.error('Logout failed'); console.error('Error during logout:', error); return; } - toast.success('Signed out!'); setIsAuthenticated(false); setRepoData(null); };Also applies to: 75-83
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/App.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/App.tsx (4)
frontend/src/lib/supabaseClient.ts (1)
supabase(12-12)frontend/src/components/pages/ForgotPasswrdPage.tsx (1)
ForgotPasswrdPage(49-153)frontend/src/components/pages/ResetPasswordPage.tsx (1)
ResetPasswordPage(50-201)frontend/src/components/pages/SignUpPage.tsx (1)
SignUpPage(51-210)
🔇 Additional comments (4)
frontend/src/App.tsx (4)
3-5: LGTM: animation and toast wiring are appropriate.Imports for AnimatePresence and react-hot-toast are correct and used properly below.
17-20: Auth pages and Supabase client imports look good.Wiring aligns with the new flows (signup/forgot/reset).
27-27: No action needed.Comment-only change.
123-139: Auth route gating looks correct.
- Redirects authenticated users away from /login, /signup, and /forgot-password.
- Leaves /reset-password public (appropriate for email links).
| const { data: subscription } = supabase.auth.onAuthStateChange( | ||
| (event, session) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify subscription handling and make cleanup safe.
Current cleanup may throw if subscription is undefined; also double “subscription.subscription” is awkward.
- const { data: subscription } = supabase.auth.onAuthStateChange(
+ const { data: { subscription } } = supabase.auth.onAuthStateChange(
(event, session) => {
// ...
}
);
- return () => {
- subscription.subscription.unsubscribe();
- };
+ return () => subscription.unsubscribe();Also applies to: 66-68
🤖 Prompt for AI Agents
In frontend/src/App.tsx around lines 38-39 (and similarly 66-68), the code
assigns const { data: subscription } = supabase.auth.onAuthStateChange(...) then
later calls subscription.subscription which is awkward and the cleanup can throw
if subscription is undefined; change to capture the return value directly (e.g.,
const subscription = supabase.auth.onAuthStateChange(...)) and in the cleanup
use optional chaining or a null-check to call subscription.unsubscribe() only if
the subscription exists (or return a cleanup function that calls
subscription?.unsubscribe()). This removes the double
“subscription.subscription” and makes teardown safe.
|
|
||
| const { data: subscription } = supabase.auth.onAuthStateChange( | ||
| (event, session) => { | ||
| console.log("Auth event:", event, session); |
There was a problem hiding this comment.
Do not log the full session object (risk of token leakage).
Access/refresh tokens can end up in browser logs. Log the event only (or at most session?.user.id).
- console.log("Auth event:", event, session);
+ console.log("Auth event:", event);📝 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.
| console.log("Auth event:", event, session); | |
| console.log("Auth event:", event); |
🤖 Prompt for AI Agents
In frontend/src/App.tsx around line 40, the current console.log prints the full
session object which may expose tokens; remove or replace that log so it does
not emit the session contents. Log only the event (e.g., console.log("Auth
event:", event)) or, if you need an identifier, log session?.user?.id or other
non-sensitive fields; ensure you do not include session.access_token,
session.refresh_token, or any token-like fields in logs.
There was a problem hiding this comment.
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 (2)
frontend/src/components/landing/LandingPage.tsx (2)
16-19: Validate GitHub repo URLs, not just non-emptyPrevents avoidable backend calls and clearer UX.
- if (!repoUrl) { - toast.error('Please enter a valid GitHub repository URL.'); - return; - } + if (!/^https?:\/\/github\.com\/[^\/\s]+\/[^\/\s]+\/?$/.test(repoUrl)) { + toast.error('Please enter a valid GitHub repository URL (owner/repo).'); + return; + }
1-6: Don’t hard-code localhost; add Supabase auth headerUse an env-configured API base to avoid mixed content in prod and include the Supabase access token if the backend protects this endpoint.
import React, { useState } from 'react'; import { motion } from 'framer-motion'; import axios from 'axios'; import { toast } from 'react-hot-toast'; import { useNavigate } from 'react-router-dom'; +import { supabase } from '../../lib/supabaseClient';
🧹 Nitpick comments (2)
frontend/src/components/landing/LandingPage.tsx (2)
7-9: Avoidanyfor repo dataType the payload to keep call-sites safe and future-proof.
-interface Props { - setRepoData: (data: any) => void; // Function to pass data to parent -} +type RepoStats = Record<string, unknown>; // TODO: refine shape +interface Props { + setRepoData: (data: RepoStats) => void; +}
41-58: Submit on Enter and add basic a11yWrap controls in a form to allow Enter key submission, and add small a11y niceties.
- <div className="bg-gray-900 p-8 rounded-lg shadow-lg w-full max-w-md"> + <div className="bg-gray-900 p-8 rounded-lg shadow-lg w-full max-w-md"> <h1 className="text-3xl font-bold mb-4">Welcome to Devr.AI</h1> <p className="text-gray-400 mb-6">Enter a GitHub repository URL to analyze its stats.</p> - <input - type="text" - placeholder="https://github.com/owner/repo" - value={repoUrl} - onChange={(e) => setRepoUrl(e.target.value)} - className="w-full p-3 rounded-lg bg-gray-800 text-white mb-4" - /> - <button - onClick={fetchRepoStats} - disabled={loading} - className={`w-full py-3 rounded-lg text-white ${loading ? 'bg-gray-700' : 'bg-green-500 hover:bg-green-600'} transition-colors`} - > - {loading ? 'Fetching...' : 'Analyze Repository'} - </button> + <form onSubmit={(e) => { e.preventDefault(); fetchRepoStats(); }}> + <input + type="url" + placeholder="https://github.com/owner/repo" + value={repoUrl} + onChange={(e) => setRepoUrl(e.target.value)} + className="w-full p-3 rounded-lg bg-gray-800 text-white mb-4" + aria-label="GitHub repository URL" + autoFocus + autoComplete="off" + inputMode="url" + /> + <button + type="submit" + disabled={loading} + aria-busy={loading} + className={`w-full py-3 rounded-lg text-white ${loading ? 'bg-gray-700' : 'bg-green-500 hover:bg-green-600'} transition-colors`} + > + {loading ? 'Fetching...' : 'Analyze Repository'} + </button> + </form> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/landing/LandingPage.tsx(2 hunks)frontend/src/components/pages/ProfilePage.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/pages/ProfilePage.tsx
🔇 Additional comments (4)
frontend/src/components/landing/LandingPage.tsx (4)
5-5: Router-based navigation import looks goodSwitching to useNavigate aligns this page with the new auth + router flow.
14-14: Inject API base URL via env
Use Vite’s import.meta.env to inject VITE_API_BASE_URL in LandingPage.tsx:const navigate = useNavigate(); + const API_BASE = import.meta.env.VITE_API_BASE_URL || '';
11-14: LandingPage prop API change verified
No remaining call-sites pass the deprecated setActivePage prop; LandingPage is correctly invoked with setRepoData, and the only setActivePage occurrence in BotIntegrationPage.tsx is a local state setter.
26-26: ProtectedLayout secures/dashboard
The/dashboardroute is nested under the root<Route path="/" element={isAuthenticated ? <ProtectedLayout/> : <Navigate to='/login'/>}>, so unauthenticated users are redirected to/loginand cannot access dashboard content.
| const response = await axios.post('http://localhost:8000/api/repo-stats', { repo_url: repoUrl }); | ||
| setRepoData(response.data); // Pass fetched data to parent | ||
| toast.success('Repository stats fetched successfully!'); | ||
| setActivePage('dashboard'); // Navigate to dashboard | ||
| navigate('/dashboard'); // Navigate to dashboard | ||
| } catch (error) { | ||
| toast.error('Failed to fetch repository stats. Please check the URL or backend server.'); | ||
| } finally { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pass auth token, add timeout, and route 401s to login
Improves security, resilience, and UX.
- const response = await axios.post('http://localhost:8000/api/repo-stats', { repo_url: repoUrl });
+ const { data: { session } } = await supabase.auth.getSession();
+ const response = await axios.post(
+ `${API_BASE}/api/repo-stats`,
+ { repo_url: repoUrl },
+ {
+ headers: session?.access_token ? { Authorization: `Bearer ${session.access_token}` } : undefined,
+ timeout: 15000,
+ }
+ );
setRepoData(response.data); // Pass fetched data to parent
toast.success('Repository stats fetched successfully!');
navigate('/dashboard'); // Navigate to dashboard
- } catch (error) {
- toast.error('Failed to fetch repository stats. Please check the URL or backend server.');
+ } catch (error) {
+ if (axios.isAxiosError(error)) {
+ if (error.response?.status === 401) {
+ toast.error('Please log in to analyze repositories.');
+ navigate('/login');
+ return;
+ }
+ const msg = error.response?.data?.message ?? error.message;
+ toast.error(`Failed to fetch repository stats: ${msg}`);
+ } else {
+ toast.error('Failed to fetch repository stats. Please check the URL or backend server.');
+ }
} finally {
setLoading(false);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/components/landing/LandingPage.tsx around lines 23 to 29, the
axios POST call should include the user's auth token, a request timeout, and
explicit handling of 401 responses; change the request to send headers: {
Authorization: `Bearer ${token}` } (obtain token from localStorage or auth
context) and add a timeout option (e.g., 10000 ms), then in the catch block
inspect the error response status: if 401 navigate('/login') and show an
appropriate toast, otherwise show the existing generic failure toast; keep
finally behavior unchanged.
3 participants
Closes #115
📝 Description
In this PR i have added a basic Authentication system for the frontend with supabases built in email authentication system.
🔧 Changes Made
📷 Screenshots or Visual Changes (if applicable)
Login Screen
SignUp Screen
After SignUp Screen
🤝 Collaboration
nil
✅ Checklist
Summary by CodeRabbit
New Features
Chores
Documentation