diff --git a/backend/alembic/versions/c9d0e1f2a3b4_draft_questions.py b/backend/alembic/versions/c9d0e1f2a3b4_draft_questions.py new file mode 100644 index 0000000..7279d95 --- /dev/null +++ b/backend/alembic/versions/c9d0e1f2a3b4_draft_questions.py @@ -0,0 +1,73 @@ +"""A staging area for extracted questions. + +Extraction wrote straight into `questions`, so a machine's first attempt took a +permanent id the moment it was produced — and ids come from a sequence that +never reissues one, so every rejected draft burned an id and every draft that +needed fixing was in the bank while it was being fixed. + +Drafts have their own table and their own sequence now. Nothing reaches the +bank, or takes a question id, until somebody accepts it. + +Revision ID: c9d0e1f2a3b4 +Revises: b8c9d0e1f2a3 +""" +import sqlalchemy as sa +from alembic import op + +revision = "c9d0e1f2a3b4" +down_revision = "b8c9d0e1f2a3" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "draft_batches", + sa.Column("id", sa.Integer(), primary_key=True, index=True), + sa.Column("title", sa.String(200), nullable=False), + sa.Column("document_id", sa.Integer(), sa.ForeignKey("pdf_documents.id", ondelete="SET NULL"), nullable=True), + sa.Column("section_id", sa.Integer(), sa.ForeignKey("sections.id", ondelete="SET NULL"), nullable=True), + sa.Column("job_id", sa.String(64), nullable=True), + sa.Column("model_id", sa.String(160), nullable=True), + sa.Column("extraction_mode", sa.String(40), nullable=True), + sa.Column("category_id", sa.Integer(), sa.ForeignKey("question_categories.id", ondelete="SET NULL"), nullable=True), + sa.Column("created_by", sa.Integer(), sa.ForeignKey("users.id", ondelete="SET NULL"), nullable=True), + sa.Column("created_at", sa.DateTime(), nullable=True), + sa.Column("status", sa.String(20), server_default="open"), + ) + op.create_index("ix_draft_batches_job_id", "draft_batches", ["job_id"]) + op.create_index("ix_draft_batches_status", "draft_batches", ["status"]) + + op.create_table( + "draft_questions", + sa.Column("id", sa.Integer(), primary_key=True, index=True), + sa.Column("batch_id", sa.Integer(), sa.ForeignKey("draft_batches.id", ondelete="CASCADE"), nullable=False), + sa.Column("position", sa.Integer(), server_default="0"), + sa.Column("question_text", sa.Text(), nullable=False), + sa.Column("question_type", sa.String(), nullable=False, server_default="mcq"), + sa.Column("options", sa.JSON(), nullable=True), + sa.Column("correct_answer", sa.String(), nullable=True), + sa.Column("explanation", sa.Text(), nullable=True), + sa.Column("option_explanations", sa.JSON(), nullable=True), + sa.Column("key_points", sa.JSON(), nullable=True), + sa.Column("attending_tip", sa.Text(), nullable=True), + sa.Column("difficulty", sa.String(10), nullable=True), + sa.Column("page_reference", sa.Integer(), nullable=True), + sa.Column("image_path", sa.String(), nullable=True), + sa.Column("explanation_image_path", sa.String(), nullable=True), + sa.Column("category_id", sa.Integer(), sa.ForeignKey("question_categories.id", ondelete="SET NULL"), nullable=True), + sa.Column("status", sa.String(20), server_default="pending"), + sa.Column("question_id", sa.Integer(), sa.ForeignKey("questions.id", ondelete="SET NULL"), nullable=True), + sa.Column("note", sa.Text(), nullable=True), + sa.Column("decided_by", sa.Integer(), sa.ForeignKey("users.id", ondelete="SET NULL"), nullable=True), + sa.Column("decided_at", sa.DateTime(), nullable=True), + sa.Column("edited", sa.Integer(), server_default="0"), + sa.Column("created_at", sa.DateTime(), nullable=True), + ) + op.create_index("ix_draft_questions_batch_id", "draft_questions", ["batch_id"]) + op.create_index("ix_draft_questions_status", "draft_questions", ["status"]) + + +def downgrade() -> None: + op.drop_table("draft_questions") + op.drop_table("draft_batches") diff --git a/backend/app/main.py b/backend/app/main.py index 50a717b..877e728 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -13,7 +13,7 @@ from app.database import engine, Base, SessionLocal from app.routers import auth, documents, quizzes, attempts, admin, tts, nextcloud, categories, questions, question_categories, favorites, teach, contact, tags, flashcards, courses, mobile, mynote, exams from app.routers import access from app.routers import feedback -from app.routers import study_tools, uploads, articles, share, collections, study_plans, media, search, ai_mode +from app.routers import study_tools, uploads, articles, share, collections, study_plans, media, search, ai_mode, drafts from app.utils.auth import get_password_hash @@ -618,6 +618,7 @@ app.include_router(access.router, prefix="/api/access", tags=["access"]) app.include_router(feedback.router, prefix="/api/feedback", tags=["feedback"]) app.include_router(exams.router, prefix="/api/exams", tags=["exams"]) app.include_router(study_plans.router, prefix="/api/study-plans", tags=["study-plans"]) +app.include_router(drafts.router, prefix="/api/drafts", tags=["drafts"]) app.include_router(media.router, prefix="/api/media", tags=["media"]) app.include_router(share.router, prefix="/api/share", tags=["share"]) app.include_router(collections.router, prefix="/api/collections", tags=["collections"]) diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index 4c7eae3..025e514 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -41,3 +41,4 @@ __all__ = [ from app.models.feedback import QuestionFeedback # noqa: F401 from app.models.invite import InviteCode # noqa: F401 +from app.models.draft_question import DraftBatch, DraftQuestion # noqa: F401 diff --git a/backend/app/models/draft_question.py b/backend/app/models/draft_question.py new file mode 100644 index 0000000..12ea467 --- /dev/null +++ b/backend/app/models/draft_question.py @@ -0,0 +1,90 @@ +from datetime import datetime + +from sqlalchemy import JSON, Column, DateTime, ForeignKey, Integer, String, Text +from sqlalchemy.orm import relationship + +from app.database import Base + + +class DraftBatch(Base): + """One run of extraction, held apart from the question bank. + + Questions pulled out of a PDF used to be written straight into `questions`, + which meant a machine's first attempt took a permanent id the moment it was + produced. Ids come from a sequence and are never reissued, so every + rejected draft burned one, and every draft that needed fixing was already + in the bank while it was being fixed. + + A batch is where a run lands instead: readable, editable, and rejectable, + with nothing in the bank until somebody says so. + """ + + __tablename__ = "draft_batches" + + id = Column(Integer, primary_key=True, index=True) + title = Column(String(200), nullable=False) + #: Where it came from, so a draft can be checked against its source. + document_id = Column(Integer, ForeignKey("pdf_documents.id", ondelete="SET NULL"), nullable=True) + section_id = Column(Integer, ForeignKey("sections.id", ondelete="SET NULL"), nullable=True) + job_id = Column(String(64), nullable=True, index=True) + #: Which model produced it, and how it was asked. Two batches from the same + #: pages can differ entirely on these, and an educator comparing them + #: should not have to guess which was which. + model_id = Column(String(160), nullable=True) + extraction_mode = Column(String(40), nullable=True) + #: Where the questions go when they are accepted. Chosen up front so a + #: batch has a home before anyone reads it. + category_id = Column(Integer, ForeignKey("question_categories.id", ondelete="SET NULL"), nullable=True) + created_by = Column(Integer, ForeignKey("users.id", ondelete="SET NULL"), nullable=True) + created_at = Column(DateTime, default=datetime.utcnow) + #: open — being worked through. closed — nothing left to decide. + status = Column(String(20), default="open", index=True) + + drafts = relationship("DraftQuestion", back_populates="batch", + cascade="all, delete-orphan", order_by="DraftQuestion.position") + + +class DraftQuestion(Base): + """A proposed question. It has an id here, and no id in the bank. + + The fields mirror `Question` because promoting one is a copy, not a + translation — anything that cannot be expressed here would be lost at the + moment of acceptance, which is the worst possible time to discover it. + """ + + __tablename__ = "draft_questions" + + id = Column(Integer, primary_key=True, index=True) + batch_id = Column(Integer, ForeignKey("draft_batches.id", ondelete="CASCADE"), nullable=False, index=True) + position = Column(Integer, default=0) + + question_text = Column(Text, nullable=False) + question_type = Column(String, nullable=False, default="mcq") + options = Column(JSON, nullable=True) + correct_answer = Column(String, nullable=True) + explanation = Column(Text, nullable=True) + option_explanations = Column(JSON, nullable=True) + key_points = Column(JSON, nullable=True) + attending_tip = Column(Text, nullable=True) + difficulty = Column(String(10), nullable=True) + page_reference = Column(Integer, nullable=True) + image_path = Column(String, nullable=True) + explanation_image_path = Column(String, nullable=True) + #: Overrides the batch's category for this one draft. + category_id = Column(Integer, ForeignKey("question_categories.id", ondelete="SET NULL"), nullable=True) + + #: pending | accepted | rejected. An accepted draft keeps its row and + #: records which question it became, so the batch still reads as a history + #: of what was decided rather than emptying as it is worked through. + status = Column(String(20), default="pending", index=True) + question_id = Column(Integer, ForeignKey("questions.id", ondelete="SET NULL"), nullable=True) + #: Why it was turned down, for whoever tunes the extraction next. + note = Column(Text, nullable=True) + decided_by = Column(Integer, ForeignKey("users.id", ondelete="SET NULL"), nullable=True) + decided_at = Column(DateTime, nullable=True) + #: Whether a human changed it before deciding — the number worth knowing + #: about a model's output. + edited = Column(Integer, default=0) + created_at = Column(DateTime, default=datetime.utcnow) + + batch = relationship("DraftBatch", back_populates="drafts") diff --git a/backend/app/routers/drafts.py b/backend/app/routers/drafts.py new file mode 100644 index 0000000..5f055c5 --- /dev/null +++ b/backend/app/routers/drafts.py @@ -0,0 +1,257 @@ +"""Extracted questions, before they are questions. + +A batch is one run of extraction. It is read, edited and decided here, and +nothing reaches the question bank — or takes a question id — until somebody +accepts it. +""" +from fastapi import APIRouter, Depends, HTTPException, Query +from pydantic import BaseModel, Field +from sqlalchemy import func +from sqlalchemy.orm import Session + +from app.database import get_db +from app.models.draft_question import DraftBatch, DraftQuestion +from app.models.question_category import QuestionCategory +from app.models.user import User +from app.services import draft_questions as drafts +from app.utils.auth import get_current_user, require_moderator +from app.utils.quiz_questions import validate_key_points, validate_option_explanations + +router = APIRouter() + + +def _counts(db: Session, batch_ids: list[int]) -> dict[int, dict[str, int]]: + if not batch_ids: + return {} + rows = (db.query(DraftQuestion.batch_id, DraftQuestion.status, func.count(DraftQuestion.id)) + .filter(DraftQuestion.batch_id.in_(batch_ids)) + .group_by(DraftQuestion.batch_id, DraftQuestion.status).all()) + out: dict[int, dict[str, int]] = {} + for batch_id, status, count in rows: + out.setdefault(batch_id, {})[status] = count + return out + + +def _get_batch(db: Session, batch_id: int) -> DraftBatch: + batch = db.get(DraftBatch, batch_id) + if not batch: + raise HTTPException(404, "Batch not found") + return batch + + +def _get_draft(db: Session, draft_id: int) -> tuple[DraftQuestion, DraftBatch]: + draft = db.get(DraftQuestion, draft_id) + if not draft: + raise HTTPException(404, "Draft not found") + return draft, _get_batch(db, draft.batch_id) + + +@router.get("/batches") +def list_batches( + status: str | None = Query(None), + db: Session = Depends(get_db), + current_user: User = Depends(require_moderator), +): + query = db.query(DraftBatch) + if status: + query = query.filter(DraftBatch.status == status) + batches = query.order_by(DraftBatch.created_at.desc()).limit(200).all() + counts = _counts(db, [b.id for b in batches]) + return [drafts.batch_json(db, b, counts.get(b.id, {})) for b in batches] + + +@router.get("/batches/{batch_id}") +def read_batch( + batch_id: int, + status: str | None = Query(None), + db: Session = Depends(get_db), + current_user: User = Depends(require_moderator), +): + batch = _get_batch(db, batch_id) + query = db.query(DraftQuestion).filter(DraftQuestion.batch_id == batch_id) + if status: + query = query.filter(DraftQuestion.status == status) + rows = query.order_by(DraftQuestion.position, DraftQuestion.id).all() + body = drafts.batch_json(db, batch, _counts(db, [batch_id]).get(batch_id, {})) + body["drafts"] = [drafts.as_json(d) for d in rows] + return body + + +class BatchEdit(BaseModel): + title: str | None = None + category_id: int | None = None + status: str | None = None + + +@router.patch("/batches/{batch_id}") +def update_batch( + batch_id: int, + data: BatchEdit, + db: Session = Depends(get_db), + current_user: User = Depends(require_moderator), +): + batch = _get_batch(db, batch_id) + fields = data.model_dump(exclude_unset=True) + if fields.get("status") and fields["status"] not in ("open", "closed"): + raise HTTPException(400, "A batch is open or closed") + if fields.get("category_id") is not None and not db.get(QuestionCategory, fields["category_id"]): + raise HTTPException(400, "Category not found") + for key, value in fields.items(): + setattr(batch, key, value) + db.commit() + return drafts.batch_json(db, batch, _counts(db, [batch_id]).get(batch_id, {})) + + +@router.delete("/batches/{batch_id}", status_code=204) +def delete_batch( + batch_id: int, + db: Session = Depends(get_db), + current_user: User = Depends(require_moderator), +): + """Throw a whole run away. Questions already accepted from it stay — they + are in the bank now and are nothing to do with the batch any more.""" + db.delete(_get_batch(db, batch_id)) + db.commit() + + +class DraftEdit(BaseModel): + question_text: str | None = None + question_type: str | None = None + options: list[str] | None = None + correct_answer: str | None = None + explanation: str | None = None + option_explanations: dict | None = None + key_points: list[dict] | None = None + attending_tip: str | None = None + difficulty: str | None = None + category_id: int | None = None + + +@router.patch("/{draft_id}") +def update_draft( + draft_id: int, + data: DraftEdit, + db: Session = Depends(get_db), + current_user: User = Depends(require_moderator), +): + """Fix a draft in place. This is the whole point of the staging table: it + can be wrong, and be corrected, without anything being in the bank.""" + draft, _ = _get_draft(db, draft_id) + if draft.status == "accepted": + raise HTTPException(409, "That draft is in the bank; edit the question instead") + fields = data.model_dump(exclude_unset=True) + if "difficulty" in fields and fields["difficulty"] not in (None, "easy", "medium", "hard"): + raise HTTPException(422, "Difficulty is easy, medium or hard") + options = fields.get("options", draft.options) + if "option_explanations" in fields: + fields["option_explanations"] = validate_option_explanations(options, fields["option_explanations"]) + if "key_points" in fields: + fields["key_points"] = validate_key_points(fields["key_points"], db) + if fields.get("category_id") is not None and not db.get(QuestionCategory, fields["category_id"]): + raise HTTPException(400, "Category not found") + for key, value in fields.items(): + setattr(draft, key, value) + # Worth knowing about a model's output: how much of it a human had to change. + draft.edited = 1 + db.commit() + return drafts.as_json(draft) + + +class Decision(BaseModel): + ids: list[int] = Field(min_length=1, max_length=200) + note: str | None = None + + +@router.post("/accept") +def accept_drafts( + data: Decision, + db: Session = Depends(get_db), + current_user: User = Depends(require_moderator), +): + """Put drafts into the bank. Each one becomes a question here and nowhere + else, which is the only moment a question id is taken. + + All or nothing: a batch half-accepted because the twentieth draft was + missing an answer is worse than a refusal naming it. + """ + rows = db.query(DraftQuestion).filter(DraftQuestion.id.in_(data.ids)).all() + missing = set(data.ids) - {row.id for row in rows} + if missing: + raise HTTPException(404, f"No such draft: {', '.join(str(m) for m in sorted(missing))}") + + batches = {b.id: b for b in db.query(DraftBatch).filter( + DraftBatch.id.in_({row.batch_id for row in rows})).all()} + + # Everything is checked before anything is created. Creating as we go and + # refusing part-way leaves questions in the bank from a call that reported + # failure — and the caller has no way to know which. + # Already in the bank is a different answer from not ready: one is a + # conflict with what has happened, the other is work still to do. + done = [row.id for row in rows if row.status == "accepted" and row.question_id] + if done: + raise HTTPException(409, "Already in the bank: " + f"{', '.join(f'draft {i}' for i in sorted(done))}") + faults = [] + for row in rows: + found = drafts.problems(row) + if found: + faults.append(f"draft {row.id}: {', '.join(found)}") + if faults: + raise HTTPException(400, "; ".join(faults[:10])) + + made = [] + try: + for row in sorted(rows, key=lambda r: (r.batch_id, r.position, r.id)): + question = drafts.accept(db, row, batches[row.batch_id], current_user) + made.append({"draft_id": row.id, "question_id": question.id}) + db.commit() + except Exception: + db.rollback() + raise + + # A batch with nothing left to decide closes itself. + for batch_id in {row.batch_id for row in rows}: + left = db.query(DraftQuestion.id).filter( + DraftQuestion.batch_id == batch_id, DraftQuestion.status == "pending").first() + if not left: + db.get(DraftBatch, batch_id).status = "closed" + db.commit() + return {"accepted": len(made), "questions": made} + + +@router.post("/reject") +def reject_drafts( + data: Decision, + db: Session = Depends(get_db), + current_user: User = Depends(require_moderator), +): + rows = db.query(DraftQuestion).filter(DraftQuestion.id.in_(data.ids)).all() + for row in rows: + drafts.reject(db, row, current_user, data.note) + db.commit() + return {"rejected": len(rows)} + + +@router.post("/{draft_id}/reopen") +def reopen_draft( + draft_id: int, + db: Session = Depends(get_db), + current_user: User = Depends(require_moderator), +): + draft, _ = _get_draft(db, draft_id) + drafts.reopen(db, draft) + db.commit() + return drafts.as_json(draft) + + +@router.delete("/{draft_id}", status_code=204) +def delete_draft( + draft_id: int, + db: Session = Depends(get_db), + current_user: User = Depends(require_moderator), +): + """Remove a draft outright. Nothing in the bank is touched: a draft that + was accepted has already become a question, which is its own row.""" + draft, _ = _get_draft(db, draft_id) + db.delete(draft) + db.commit() diff --git a/backend/app/routers/quizzes.py b/backend/app/routers/quizzes.py index 6a95b75..6ba6ea1 100644 --- a/backend/app/routers/quizzes.py +++ b/backend/app/routers/quizzes.py @@ -105,14 +105,19 @@ def list_user_jobs(current_user: User = Depends(get_current_user)): last_step = "" if steps: last_step = _json.loads(steps[-1]).get("message", "") + # Extraction lands in a review batch now rather than a quiz. quiz_id + # is still read for jobs that ran before that change and whose keys are + # still in Redis. + batch_id = r.get(f"extraction:batch_id:{jid}") quiz_id = r.get(f"extraction:quiz_id:{jid}") - title = r.get(f"extraction:job_title:{jid}") or "Quiz" + title = r.get(f"extraction:job_title:{jid}") or "Extraction" jobs.append({ "job_id": jid, "title": title, "status": status, "steps_count": len(steps), "last_step": last_step[:80], + "batch_id": int(batch_id) if batch_id else None, "quiz_id": int(quiz_id) if quiz_id else None, }) return jobs @@ -120,7 +125,8 @@ def list_user_jobs(current_user: User = Depends(get_current_user)): @router.get("/job/{job_id}") def get_extraction_job(job_id: str, current_user: User = Depends(require_moderator)): - """Poll extraction job progress. Returns steps list, status, and quiz_id when done.""" + """Poll extraction job progress. Returns the steps, the status, and — when it + finishes — the review batch the drafts landed in.""" import json as _json import redis as redis_lib from app.config import settings @@ -131,7 +137,11 @@ def get_extraction_job(job_id: str, current_user: User = Depends(require_moderat steps = [_json.loads(s) for s in raw_steps] result = {"job_id": job_id, "status": status, "steps": steps} if status == "completed": - result["quiz_id"] = int(r.get(f"extraction:quiz_id:{job_id}") or 0) + batch_id = r.get(f"extraction:batch_id:{job_id}") + result["batch_id"] = int(batch_id) if batch_id else None + # Jobs that ran before extraction was staged still point at a quiz. + quiz_id = r.get(f"extraction:quiz_id:{job_id}") + result["quiz_id"] = int(quiz_id) if quiz_id else None if status == "failed": result["error"] = r.get(f"extraction:error:{job_id}") or "Unknown error" return result diff --git a/backend/app/services/draft_questions.py b/backend/app/services/draft_questions.py new file mode 100644 index 0000000..d236d14 --- /dev/null +++ b/backend/app/services/draft_questions.py @@ -0,0 +1,147 @@ +"""Drafts, and the one moment they become questions. + +The rule the whole arrangement exists for: a draft has no question id. Ids come +from a sequence and are never reissued, so a machine's first attempt taking one +the moment it is produced means every rejected draft burns an id, and every +draft being fixed is in the bank while it is being fixed. + +Accepting is therefore the only place a `Question` is created, and it is a copy +rather than a translation — every field a draft can hold is a field a question +has, so nothing is lost at the moment of acceptance. +""" +from datetime import datetime + +from fastapi import HTTPException +from sqlalchemy.orm import Session + +from app.models.draft_question import DraftBatch, DraftQuestion +from app.models.question import Question +from app.models.question_category import QuestionCategory +from app.models.user import User + +#: What a question needs before anyone can sit it. Checked at acceptance rather +#: than at extraction, because a draft is allowed to be incomplete — that is +#: what it is for. +def problems(draft: DraftQuestion) -> list[str]: + found = [] + if not (draft.question_text or "").strip(): + found.append("no question text") + if draft.question_type == "mcq": + options = draft.options or [] + if len(options) < 2: + found.append("fewer than two options") + elif not draft.correct_answer: + found.append("no correct answer marked") + elif draft.correct_answer not in options: + found.append("the correct answer is not one of the options") + elif not (draft.correct_answer or "").strip(): + found.append("no correct answer") + return found + + +def category_for(draft: DraftQuestion, batch: DraftBatch) -> int | None: + """A draft's own category if it has one, otherwise the batch's.""" + return draft.category_id or batch.category_id + + +def accept(db: Session, draft: DraftQuestion, batch: DraftBatch, user: User) -> Question: + """Copy one draft into the bank. This is where the id is taken.""" + if draft.status == "accepted" and draft.question_id: + raise HTTPException(409, "That draft has already been accepted") + faults = problems(draft) + if faults: + raise HTTPException(400, f"Not ready: {', '.join(faults)}") + + category_id = category_for(draft, batch) + if category_id is not None and not db.get(QuestionCategory, category_id): + raise HTTPException(400, "The category this batch files into no longer exists") + + question = Question( + question_category_id=category_id, + question_text=draft.question_text.strip(), + question_type=draft.question_type, + options=draft.options, + correct_answer=draft.correct_answer, + explanation=draft.explanation, + option_explanations=draft.option_explanations, + key_points=draft.key_points, + attending_tip=draft.attending_tip, + difficulty=draft.difficulty, + page_reference=draft.page_reference, + image_path=draft.image_path, + explanation_image_path=draft.explanation_image_path, + user_id=user.id, + is_shared=1, + ) + db.add(question) + db.flush() + + # The draft keeps its row and records what it became, so the batch reads as + # a history of what was decided rather than emptying as it is worked through. + draft.status = "accepted" + draft.question_id = question.id + draft.decided_by = user.id + draft.decided_at = datetime.utcnow() + return question + + +def reject(db: Session, draft: DraftQuestion, user: User, note: str | None = None) -> None: + if draft.status == "accepted": + raise HTTPException(409, "That draft is already in the bank") + draft.status = "rejected" + draft.note = (note or "").strip() or None + draft.decided_by = user.id + draft.decided_at = datetime.utcnow() + + +def reopen(db: Session, draft: DraftQuestion) -> None: + """Undo a rejection. An accepted draft cannot be reopened — the question + exists, and deciding again would make a second one.""" + if draft.status == "accepted": + raise HTTPException(409, "That draft is in the bank; edit or delete the question instead") + draft.status = "pending" + draft.note = None + draft.decided_by = None + draft.decided_at = None + + +def as_json(draft: DraftQuestion) -> dict: + return { + "id": draft.id, + "position": draft.position, + "question_text": draft.question_text, + "question_type": draft.question_type, + "options": draft.options, + "correct_answer": draft.correct_answer, + "explanation": draft.explanation, + "option_explanations": draft.option_explanations, + "key_points": draft.key_points, + "attending_tip": draft.attending_tip, + "difficulty": draft.difficulty, + "page_reference": draft.page_reference, + "image_path": draft.image_path, + "explanation_image_path": draft.explanation_image_path, + "category_id": draft.category_id, + "status": draft.status, + "question_id": draft.question_id, + "note": draft.note, + "edited": bool(draft.edited), + # Said for every draft, not only on the attempt to accept it, so a + # reviewer can see what needs work before opening anything. + "problems": problems(draft), + } + + +def batch_json(db: Session, batch: DraftBatch, counts: dict[str, int] | None = None) -> dict: + return { + "id": batch.id, + "title": batch.title, + "document_id": batch.document_id, + "section_id": batch.section_id, + "model_id": batch.model_id, + "extraction_mode": batch.extraction_mode, + "category_id": batch.category_id, + "status": batch.status, + "created_at": batch.created_at, + "counts": counts or {}, + } diff --git a/backend/app/tasks/quiz_tasks.py b/backend/app/tasks/quiz_tasks.py index c38ce17..3ccd39d 100644 --- a/backend/app/tasks/quiz_tasks.py +++ b/backend/app/tasks/quiz_tasks.py @@ -272,20 +272,27 @@ def extract_quiz( except Exception as e: _push_step(r, job_id, "images", f"Image extraction skipped: {e}") - # Create quiz - quiz = Quiz( - section_id=section_id, - user_id=user_id, + # A batch, not a quiz. What a model pulled out of a PDF is a proposal: + # it is read, corrected and decided before it is anything, and a + # question id — which comes from a sequence and is never reissued — is + # taken at acceptance rather than at extraction. + from app.models.draft_question import DraftBatch, DraftQuestion + + batch = DraftBatch( title=title, - questions_count=len(valid_questions), - mode=mode, - time_limit_minutes=time_limit_minutes, - skipped_questions=json.dumps(skipped) if skipped else None, + document_id=document.id, + section_id=section_id, + job_id=job_id, + model_id=model_id, + extraction_mode=extraction_mode, + category_id=question_category_id, + created_by=user_id, + status="open", ) - db.add(quiz) + db.add(batch) db.flush() - _push_step(r, job_id, "save", f"Saving {len(valid_questions)} questions and generating embeddings…") + _push_step(r, job_id, "save", f"Saving {len(valid_questions)} drafts for review…") for pos, q in enumerate(valid_questions): page_ref = q.get("page_reference") @@ -296,9 +303,9 @@ def extract_quiz( if not page_images[page_ref]: del page_images[page_ref] - question = Question( - source_quiz_id=quiz.id, - question_category_id=question_category_id, + db.add(DraftQuestion( + batch_id=batch.id, + position=pos, question_text=q["question_text"], question_type=q["question_type"], options=q.get("options"), @@ -306,22 +313,18 @@ def extract_quiz( explanation=q.get("explanation", ""), page_reference=page_ref, image_path=image_path, - ) - db.add(question) - db.flush() - db.add(QuizQuestionLink(quiz_id=quiz.id, question_id=question.id, position=pos)) - try: - embedding_service.embed_question(question) - except Exception as e: - logger.warning(f"Embedding failed for question {question.id}: {e}") + )) + # No embedding here. A vector is for finding a question in the bank, + # and a draft is not in the bank; it is generated when one is accepted. db.commit() - db.refresh(quiz) + db.refresh(batch) - _push_step(r, job_id, "done", f"Quiz ready! {len(valid_questions)} questions extracted and saved.") + _push_step(r, job_id, "done", + f"{len(valid_questions)} drafts ready to review. Nothing is in the bank yet.") r.set(f"extraction:status:{job_id}", "completed", ex=EXPIRE_SECONDS) - r.set(f"extraction:quiz_id:{job_id}", str(quiz.id), ex=EXPIRE_SECONDS) - return quiz.id + r.set(f"extraction:batch_id:{job_id}", str(batch.id), ex=EXPIRE_SECONDS) + return batch.id except Exception as e: logger.exception(f"Quiz extraction failed for job {job_id}") diff --git a/backend/tests/test_draft_questions.py b/backend/tests/test_draft_questions.py new file mode 100644 index 0000000..2d92855 --- /dev/null +++ b/backend/tests/test_draft_questions.py @@ -0,0 +1,163 @@ +"""Extracted questions are staged, and only acceptance takes a question id. + +The property the whole table exists for: a machine's first attempt must not +consume a permanent id. Ids come from a sequence and are never reissued, so +rejecting a draft that had already taken one burns it, and a draft being +corrected would be sitting in the bank while it was wrong. +""" +import unittest + +from fastapi import FastAPI +from fastapi.testclient import TestClient +from sqlalchemy import create_engine +from sqlalchemy.orm import Session +from sqlalchemy.pool import StaticPool + +import test_quiz_builder # noqa: F401 — imports every model +from app.database import Base, get_db +from app.models.draft_question import DraftBatch, DraftQuestion +from app.models.question import Question +from app.models.question_category import QuestionCategory +from app.models.user import User +from app.routers import drafts +from app.utils.auth import get_current_user + + +class DraftTests(unittest.TestCase): + def setUp(self): + self.engine = create_engine("sqlite://", connect_args={"check_same_thread": False}, + poolclass=StaticPool) + Base.metadata.create_all(self.engine) + self.db = Session(self.engine) + self.mod = User(id=1, name="Mod", email="m@example.test", + hashed_password="unused", role="moderator") + self.learner = User(id=2, name="Learner", email="l@example.test", + hashed_password="unused", role="user") + self.db.add_all([self.mod, self.learner]) + self.db.add(QuestionCategory(id=5, name="Cardiology", user_id=1)) + self.batch = DraftBatch(id=1, title="Nelson ch. 12", category_id=5, created_by=1, status="open") + self.db.add(self.batch) + self.db.flush() + self.db.add_all([ + DraftQuestion(id=1, batch_id=1, position=0, question_text="A neonate with cyanosis…", + question_type="mcq", options=["TGA", "ASD"], correct_answer="TGA", + explanation="Because."), + DraftQuestion(id=2, batch_id=1, position=1, question_text="Incomplete one", + question_type="mcq", options=["only one"], correct_answer=None), + DraftQuestion(id=3, batch_id=1, position=2, question_text="Answer not an option", + question_type="mcq", options=["a", "b"], correct_answer="c"), + ]) + self.db.commit() + + self.user = self.mod + app = FastAPI() + app.include_router(drafts.router, prefix="/drafts") + app.dependency_overrides[get_db] = lambda: self.db + app.dependency_overrides[get_current_user] = lambda: self.user + self.client = TestClient(app) + + def tearDown(self): + self.client.close() + self.db.close() + self.engine.dispose() + + def test_a_draft_holds_no_question_id_until_it_is_accepted(self): + self.assertEqual(self.db.query(Question).count(), 0) + body = self.client.get("/drafts/batches/1").json() + self.assertEqual(len(body["drafts"]), 3) + self.assertTrue(all(d["question_id"] is None for d in body["drafts"])) + + def test_accepting_creates_the_question_and_records_what_it_became(self): + body = self.client.post("/drafts/accept", json={"ids": [1]}).json() + self.assertEqual(body["accepted"], 1) + question_id = body["questions"][0]["question_id"] + question = self.db.get(Question, question_id) + self.assertEqual(question.question_text, "A neonate with cyanosis…") + # Filed where the batch said, without anyone choosing again. + self.assertEqual(question.question_category_id, 5) + self.db.expire_all() + self.assertEqual(self.db.get(DraftQuestion, 1).question_id, question_id) + self.assertEqual(self.db.get(DraftQuestion, 1).status, "accepted") + + def test_rejecting_leaves_the_bank_untouched(self): + self.assertEqual(self.client.post("/drafts/reject", + json={"ids": [2], "note": "not a question"}).status_code, 200) + self.assertEqual(self.db.query(Question).count(), 0) + self.db.expire_all() + self.assertEqual(self.db.get(DraftQuestion, 2).note, "not a question") + + def test_a_draft_that_is_not_ready_is_refused_by_name(self): + response = self.client.post("/drafts/accept", json={"ids": [2]}) + self.assertEqual(response.status_code, 400) + self.assertIn("two options", response.json()["detail"]) + self.assertEqual(self.db.query(Question).count(), 0) + + def test_an_answer_that_is_not_one_of_the_options_is_caught(self): + response = self.client.post("/drafts/accept", json={"ids": [3]}) + self.assertEqual(response.status_code, 400) + self.assertIn("not one of the options", response.json()["detail"]) + + def test_accepting_a_batch_is_all_or_nothing(self): + # Half a batch accepted because the second draft was broken is worse + # than a refusal that names it. + response = self.client.post("/drafts/accept", json={"ids": [1, 2]}) + self.assertEqual(response.status_code, 400) + # Names the one that stopped it, so it can be fixed without guessing. + self.assertIn("draft 2", response.json()["detail"]) + self.assertEqual(self.db.query(Question).count(), 0) + self.db.expire_all() + self.assertEqual(self.db.get(DraftQuestion, 1).status, "pending") + + def test_problems_are_named_before_anyone_opens_a_draft(self): + body = self.client.get("/drafts/batches/1").json() + by_id = {d["id"]: d for d in body["drafts"]} + self.assertEqual(by_id[1]["problems"], []) + self.assertTrue(by_id[2]["problems"]) + + def test_editing_a_draft_fixes_it_without_anything_being_in_the_bank(self): + response = self.client.patch("/drafts/2", json={ + "options": ["one", "two"], "correct_answer": "two"}) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["problems"], []) + self.assertTrue(response.json()["edited"]) + self.assertEqual(self.db.query(Question).count(), 0) + self.assertEqual(self.client.post("/drafts/accept", json={"ids": [2]}).status_code, 200) + + def test_an_accepted_draft_cannot_be_edited_or_accepted_again(self): + self.client.post("/drafts/accept", json={"ids": [1]}) + self.assertEqual(self.client.patch("/drafts/1", json={"explanation": "x"}).status_code, 409) + self.assertEqual(self.client.post("/drafts/accept", json={"ids": [1]}).status_code, 409) + self.assertEqual(self.db.query(Question).count(), 1, "no second question") + + def test_a_rejection_can_be_undone_but_an_acceptance_cannot(self): + self.client.post("/drafts/reject", json={"ids": [2]}) + self.assertEqual(self.client.post("/drafts/2/reopen").status_code, 200) + self.db.expire_all() + self.assertEqual(self.db.get(DraftQuestion, 2).status, "pending") + + self.client.post("/drafts/accept", json={"ids": [1]}) + self.assertEqual(self.client.post("/drafts/1/reopen").status_code, 409) + + def test_a_batch_closes_itself_when_nothing_is_left_to_decide(self): + self.client.post("/drafts/reject", json={"ids": [2, 3]}) + self.db.expire_all() + self.assertEqual(self.db.get(DraftBatch, 1).status, "open") + self.client.post("/drafts/accept", json={"ids": [1]}) + self.db.expire_all() + self.assertEqual(self.db.get(DraftBatch, 1).status, "closed") + + def test_deleting_a_batch_leaves_the_questions_it_produced(self): + self.client.post("/drafts/accept", json={"ids": [1]}) + self.assertEqual(self.client.delete("/drafts/batches/1").status_code, 204) + self.assertEqual(self.db.query(DraftQuestion).count(), 0) + # The question is in the bank and is nothing to do with the batch now. + self.assertEqual(self.db.query(Question).count(), 1) + + def test_none_of_this_is_a_learner_s(self): + self.user = self.learner + self.assertEqual(self.client.get("/drafts/batches").status_code, 403) + self.assertEqual(self.client.post("/drafts/accept", json={"ids": [1]}).status_code, 403) + + +if __name__ == "__main__": + unittest.main()