feat: extracted questions are staged, and only acceptance takes an id
Extraction wrote straight into `questions`, so a machine's first attempt took a permanent id the moment it was produced. Ids come from a sequence and are never reissued: every rejected draft burned one, and every draft that needed fixing was sitting in the bank while it was being fixed. A run now lands in a batch of drafts with their own table and their own sequence. They are read, corrected and decided there, and `accept` is the only place a Question is created — a copy rather than a translation, because every field a draft holds is a field a question has, so nothing is lost at the moment of acceptance. Accepting is all or nothing, and everything is checked before anything is created: a call that reports failure must not leave questions behind from the drafts it got through first. My own test caught that — the first question existed before the second draft was refused. Readiness is reported for every draft rather than only on the attempt to accept it, so a reviewer sees what needs work before opening anything. A decided draft keeps its row and records what it became, so a batch reads as a history of what was decided rather than emptying as it is worked through. An acceptance cannot be undone from here: the question exists, and deciding twice would make a second one. No embeddings for drafts. A vector is for finding a question in the bank, and a draft is not in the bank. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN
This commit is contained in:
parent
0fa5598aad
commit
c9abc4f310
9 changed files with 774 additions and 29 deletions
73
backend/alembic/versions/c9d0e1f2a3b4_draft_questions.py
Normal file
73
backend/alembic/versions/c9d0e1f2a3b4_draft_questions.py
Normal file
|
|
@ -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")
|
||||
|
|
@ -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"])
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
90
backend/app/models/draft_question.py
Normal file
90
backend/app/models/draft_question.py
Normal file
|
|
@ -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")
|
||||
257
backend/app/routers/drafts.py
Normal file
257
backend/app/routers/drafts.py
Normal file
|
|
@ -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()
|
||||
|
|
@ -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
|
||||
|
|
|
|||
147
backend/app/services/draft_questions.py
Normal file
147
backend/app/services/draft_questions.py
Normal file
|
|
@ -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 {},
|
||||
}
|
||||
|
|
@ -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}")
|
||||
|
|
|
|||
163
backend/tests/test_draft_questions.py
Normal file
163
backend/tests/test_draft_questions.py
Normal file
|
|
@ -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()
|
||||
Loading…
Reference in a new issue