Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion backend/app/routers/papers.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,12 @@ async def create_paper(
# 4. Enqueue background task
redis = await create_pool(RedisSettings.from_dsn(settings.REDIS_URL))
try:
job = await redis.enqueue_job("generate_paper_task", str(new_paper.id), str(new_job.id))
job = await redis.enqueue_job(
"generate_paper_task",
str(new_paper.id),
str(new_job.id),
_job_id=str(new_job.id)
)
if job is None:
raise RuntimeError("Failed to enqueue generate_paper_task")
await db.commit()
Expand Down
80 changes: 46 additions & 34 deletions backend/app/routers/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,46 +88,58 @@ async def upload_resource(
detail="Failed to upload file to storage"
)

# Create DB record (not committed yet)
new_resource = Resource(
user_id=current_user.id,
filename=file.filename,
file_url=file_url,
type=type,
status="processing"
)
db.add(new_resource)
await db.flush() # Flush to get the ID but don't commit

from ..models.job import Job
new_job = Job(
user_id=current_user.id,
job_type="ingest",
status="queued",
ref_id=new_resource.id
)
db.add(new_job)
await db.flush()

# Enqueue background extraction task before committing DB
try:
redis = await create_pool(RedisSettings.from_dsn(settings.REDIS_URL))
# Pass job_id as second argument
await redis.enqueue_job('extraction_task', str(new_resource.id), str(new_job.id))
# Create DB record (not committed yet)
new_resource = Resource(
user_id=current_user.id,
filename=file.filename,
file_url=file_url,
type=type,
status="processing"
)
db.add(new_resource)
await db.flush() # Flush to get the ID

# Only commit if enqueue was successful
await db.commit()
await db.refresh(new_resource)
from ..models.job import Job
new_job = Job(
user_id=current_user.id,
job_type="ingest",
status="queued",
ref_id=new_resource.id
)
db.add(new_job)
await db.flush()

# Enqueue background extraction task
redis = await create_pool(RedisSettings.from_dsn(settings.REDIS_URL))
try:
# Use _job_id to make it easy to find in hooks
await redis.enqueue_job(
'extraction_task',
str(new_resource.id),
str(new_job.id),
_job_id=str(new_job.id)
)
await db.commit()
await db.refresh(new_resource)
except Exception as e:
await db.rollback()
raise e # Caught by outer try/except
Comment on lines +129 to +133
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don’t delete storage files after a successful DB commit.

If Line 123 commit succeeds but Line 124 refresh fails, current logic still hits cleanup at Line 135 and deletes the uploaded file, leaving a committed DB record pointing to missing storage.

Suggested fix
-    try:
+    committed = False
+    try:
...
-            await db.commit()
+            await db.commit()
+            committed = True
             await db.refresh(new_resource)
...
     except Exception as e:
         # CLEANUP: Delete from storage if DB transaction failed
         logger.error(f"Upload transaction failed, cleaning up storage: {e}")
-        storage_service.delete_file(object_name)
+        if not committed:
+            storage_service.delete_file(object_name)

Also applies to: 133-136

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/routers/resources.py` around lines 123 - 127, The cleanup
currently runs regardless of whether the DB commit+refresh succeeded, which can
delete the uploaded storage file while the DB record exists; modify the handler
around db.commit() and db.refresh(new_resource) (the try/except/finally block
that also calls the storage deletion code) to track success: set a local flag
(e.g., commit_and_refresh_ok = False), set it to True only after await
db.commit() and await db.refresh(new_resource) complete without error, and then
in the cleanup/finally section only delete the uploaded file(s) if
commit_and_refresh_ok is False (i.e., rollback or error path); keep existing
rollback and re-raise behavior intact so errors still propagate.

finally:
if 'redis' in locals():
await redis.close()

except Exception as e:
await db.rollback()
# Should also ideally delete the file from Spaces here if we were strict
# CLEANUP: Delete from storage if DB transaction failed
logger.error(f"Upload transaction failed, cleaning up storage: {e}")
storage_service.delete_file(object_name)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if isinstance(e, HTTPException):
raise e
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail=f"Failed to queue background task: {str(e)}"
detail=f"Failed to complete upload transaction: {str(e)}"
)
Comment on lines 145 to 148
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid returning raw internal exception text to clients.

Line 141 exposes internal error details ({str(e)}) in a 500 response.

Suggested fix
         raise HTTPException(
             status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
-            detail=f"Failed to complete upload transaction: {str(e)}"
+            detail="Failed to complete upload transaction."
         )
🧰 Tools
🪛 Ruff (0.15.13)

[warning] 139-142: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


[warning] 141-141: Use explicit conversion flag

Replace with conversion flag

(RUF010)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/routers/resources.py` around lines 139 - 142, The handler
currently raises
HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=f"Failed
to complete upload transaction: {str(e)}") which leaks internal exception text
(variable e) to clients; instead, log the full exception internally (e.g.,
logger.exception or logging.exception with the context in the same function
where the raise appears) and change the HTTPException detail to a generic
message like "Failed to complete upload transaction" or "Internal server error
while completing upload transaction" while preserving the 500 status; ensure you
reference the same raise HTTPException and status.HTTP_500_INTERNAL_SERVER_ERROR
locations and remove direct inclusion of str(e) from the client-facing detail.

finally:
if 'redis' in locals():
await redis.close()

return new_resource

Expand Down
84 changes: 81 additions & 3 deletions backend/app/workers/arq_worker.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,93 @@
import asyncio
import logging
import sys
from datetime import datetime
from sqlalchemy import select
from arq.connections import RedisSettings
from arq.cron import cron
from app.config import settings
from ..config import settings
from .tasks import extraction_task, generate_paper_task, reset_monthly_quotas
from ..database import SessionLocal
from ..models.job import Job
from ..models.resource import Resource
from ..models.paper import Paper

# Use the arq.worker logger so messages show up in the same stream
logger = logging.getLogger('arq.worker')

async def ping(ctx):
return "pong"

async def startup(ctx):
pass
logger.info("🚀 [WORKER] Starting up and initializing hooks...")

async def shutdown(ctx):
pass
logger.info("🛑 [WORKER] Shutting down...")

async def on_job_start(ctx):
logger.info(f"▶️ [HOOK] Job Start: {ctx.get('job_id')}")

async def after_job_end(ctx):
"""
Global hook called after every job in arq.
Ensures DB status is updated even if the task itself crashed.
"""
job_id = ctx.get('job_id')
success = ctx.get('success')

# ARQ logs the failure, but we want our own trace
logger.info(f"🔍 [HOOK] Job {job_id} ended. Success: {success}")
sys.stdout.flush()

if not job_id:
logger.warning("⚠️ [HOOK] No job_id in context.")
return

async with SessionLocal() as db:
try:
# We explicitly set _job_id in routers to match our DB Job ID
# So job_id here SHOULD be the UUID of our Job record.
result = await db.execute(select(Job).where(Job.id == job_id))
job = result.scalar_one_or_none()

if not job:
logger.error(f"❌ [HOOK] Job {job_id} not found in database.")
return

logger.info(f"📊 [HOOK] Current DB Job Status: {job.status}")

# If the job is still in a transient state, force it to its final state
if job.status in ["running", "queued", "pending"]:
job.status = "done" if success else "failed"
job.completed_at = datetime.utcnow()

# Update the specific entity status (Resource or Paper)
if not success:
if job.job_type == "ingest" and job.ref_id:
res_result = await db.execute(select(Resource).where(Resource.id == job.ref_id))
resource = res_result.scalar_one_or_none()
if resource and resource.status == "processing":
resource.status = "failed"
logger.info(f"📉 [HOOK] Marked Resource {job.ref_id} as FAILED")

elif job.job_type == "generate_paper" and job.ref_id:
paper_result = await db.execute(select(Paper).where(Paper.id == job.ref_id))
paper = paper_result.scalar_one_or_none()
if paper and paper.status in ["pending", "generating"]:
paper.status = "failed"
logger.info(f"📉 [HOOK] Marked Paper {job.ref_id} as FAILED")

await db.commit()
logger.info(f"✅ [HOOK] Finalized DB Job {job_id} as {job.status}")
else:
logger.info(f"ℹ️ [HOOK] DB Job {job_id} was already finalized as {job.status}")

except Exception as e:
logger.error(f"❌ [HOOK] Error updating database for job {job_id}: {e}")
import traceback
logger.error(traceback.format_exc())
finally:
sys.stdout.flush()

class WorkerSettings:
functions = [ping, extraction_task, generate_paper_task]
Expand All @@ -20,4 +96,6 @@ class WorkerSettings:
]
on_startup = startup
on_shutdown = shutdown
on_job_start = on_job_start
after_job_end = after_job_end
redis_settings = RedisSettings.from_dsn(settings.REDIS_URL)
2 changes: 2 additions & 0 deletions backend/app/workers/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

async def extraction_task(ctx, resource_id: str, job_id: str = None):
print(f"\n🚀 [TASK START] Resource ID: {resource_id}")

# raise RuntimeError("Simulated Worker Crash")

async with SessionLocal() as db:
try:
Expand Down
21 changes: 19 additions & 2 deletions frontend/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Resources from './pages/Resources';
import Solver from './pages/Solver';
import Generator from './pages/Generator';
import ProtectedRoute from './components/ProtectedRoute';
import PublicRoute from './components/PublicRoute';
import Layout from './components/Layout';
import { useAuthStore } from './store/authStore';

Expand Down Expand Up @@ -39,6 +40,7 @@ import {
ArrowRight
} from "lucide-react"
import { cn } from './lib/utils';
import { WelcomeBanner } from './components/WelcomeBanner';

const queryClient = new QueryClient();

Expand Down Expand Up @@ -110,6 +112,7 @@ function Dashboard() {

return (
<div className="max-w-6xl mx-auto space-y-8 animate-in fade-in duration-500 pb-20">
<WelcomeBanner />
<div className="flex flex-col gap-2">
<h1 className="text-4xl font-bold tracking-tight flex items-center gap-3 text-foreground">
<LayoutDashboard className="size-9 text-primary" />
Expand Down Expand Up @@ -357,8 +360,22 @@ function App() {
<Toaster position="top-right" reverseOrder={false} />
<Router>
<Routes>
<Route path="/login" element={<Login />} />
<Route path="/register" element={<Register />} />
<Route
path="/login"
element={
<PublicRoute>
<Login />
</PublicRoute>
}
/>
<Route
path="/register"
element={
<PublicRoute>
<Register />
</PublicRoute>
}
/>

{/* Protected Routes with Layout */}
<Route
Expand Down
12 changes: 12 additions & 0 deletions frontend/src/components/PublicRoute.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { Navigate } from 'react-router-dom';
import { useAuthStore } from '../store/authStore';

export default function PublicRoute({ children }: { children: React.ReactNode }) {
const token = useAuthStore((state) => state.token);

if (token) {
return <Navigate to="/" replace />;
}

return <>{children}</>;
}
84 changes: 84 additions & 0 deletions frontend/src/components/UpgradeModal.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import {
Dialog,
DialogContent,
DialogDescription,
DialogHeader,
DialogTitle,
DialogFooter,
} from "./ui/dialog";
import { Button } from "./ui/button";
import { CheckCircle2, Zap, FileText, FileEdit, Crown } from "lucide-react";

interface UpgradeModalProps {
isOpen: boolean;
onClose: () => void;
message?: string;
}

export function UpgradeModal({ isOpen, onClose, message }: UpgradeModalProps) {
const benefits = [
{
icon: <Zap className="size-4 text-yellow-500" />,
text: "Unlimited AI Tutor Questions"
},
{
icon: <FileEdit className="size-4 text-purple-500" />,
text: "Unlimited Mock Exam Generations"
},
{
icon: <FileText className="size-4 text-blue-500" />,
text: "Unlimited Study Resource Storage"
},
{
icon: <Crown className="size-4 text-primary" />,
text: "Priority AI Model Access"
}
];

return (
<Dialog open={isOpen} onOpenChange={onClose}>
<DialogContent className="sm:max-w-[450px] overflow-hidden border-primary/20">
<div className="absolute top-0 left-0 w-full h-1.5 bg-primary" />

<DialogHeader className="pt-4">
<div className="size-12 bg-primary/10 rounded-full flex items-center justify-center mb-4">
<Crown className="size-6 text-primary" />
</div>
<DialogTitle className="text-2xl font-black tracking-tight">Upgrade to Premium</DialogTitle>
<DialogDescription className="text-base pt-2">
{message || "You've reached your free tier limit. Upgrade now to unlock the full power of PYQ Gen."}
</DialogDescription>
</DialogHeader>

<div className="py-6 space-y-4">
<h4 className="text-sm font-bold uppercase tracking-widest text-muted-foreground">Premium Benefits:</h4>
<div className="grid grid-cols-1 gap-3">
{benefits.map((benefit, i) => (
<div key={i} className="flex items-center gap-3 p-3 rounded-xl bg-muted/30 border border-border/50">
<div className="shrink-0">{benefit.icon}</div>
<span className="text-sm font-medium text-foreground">{benefit.text}</span>
<CheckCircle2 className="size-4 text-primary ml-auto" />
</div>
))}
</div>
</div>

<DialogFooter className="flex-col sm:flex-col gap-3">
<Button
className="w-full h-12 text-base font-bold shadow-lg shadow-primary/20"
onClick={() => {
// Future: Redirect to checkout or open Razorpay
console.log("Redirecting to payment...");
onClose();
}}
>
Upgrade Now — $9/mo
</Button>
Comment on lines +67 to +76
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Wire the primary CTA to an actual upgrade action.

Line 71 only logs and closes the dialog, so “Upgrade Now” does not upgrade or redirect users.

💡 Suggested fix
 interface UpgradeModalProps {
   isOpen: boolean;
   onClose: () => void;
+  onUpgrade: () => void;
   message?: string;
 }
 
-export function UpgradeModal({ isOpen, onClose, message }: UpgradeModalProps) {
+export function UpgradeModal({ isOpen, onClose, onUpgrade, message }: UpgradeModalProps) {
@@
           <Button 
             className="w-full h-12 text-base font-bold shadow-lg shadow-primary/20"
             onClick={() => {
-              // Future: Redirect to checkout or open Razorpay
-              console.log("Redirecting to payment...");
-              onClose();
+              onUpgrade();
             }}
           >
             Upgrade Now — $9/mo
           </Button>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/UpgradeModal.tsx` around lines 67 - 76, The Button in
UpgradeModal.tsx currently only logs and calls onClose in its onClick handler;
replace the console.log with a real upgrade flow by invoking a dedicated upgrade
function (e.g., a prop like onUpgrade or a local helper such as
initiateCheckout/initiateRazorpay) from the Button's onClick, handle async
errors and loading state (disable the Button while pending), and only call
onClose after a successful redirect/checkout initiation; update the UpgradeModal
component signature to accept an onUpgrade prop if needed and use
Button/onClick/onClose to trigger that flow.

<Button variant="ghost" onClick={onClose} className="w-full font-semibold">
Maybe Later
</Button>
</DialogFooter>
</DialogContent>
</Dialog>
);
}
Loading