Compare commits

..
Author SHA1 Message Date
kcar 4f66fb83ce Merge fx-3-image-only-projection
api-ci-deploy / test-build-deploy (push) Has been cancelled
2026-07-02 21:48:08 +00:00
kcar a0f77333b9 Merge fx-2-born-digital-marks 2026-07-02 21:48:08 +00:00
kcar e4b0477b77 Merge fx-1-auto-map-pk 2026-07-02 21:48:08 +00:00
kcar e95b148e0f Merge FX-7: marking completion status + max_marks validation 2026-07-02 21:48:01 +00:00
kcar b0b59077bc Merge FX-6: seed SpecPoint catalogue for all 6 test specs 2026-07-02 21:48:01 +00:00
CC WorkerandClaude Opus 4.8 931e254b93 FX-7: marking completion status + max_marks validation
batches.py upsert_mark previously never advanced a submission or batch to
'complete', and never validated an award against the question's max.

- Reject (422) an awarded_marks that exceeds the question's max_marks — only when
  a max is actually set (0/None = not-yet-scored AI/unmapped question, unvalidatable).
- _advance_completion: a submission with a mark for every markable (leaf) question
  → 'complete'; a batch whose every non-absent submission is complete → 'complete'.
  Container questions and absent students are excluded; the helper only promotes,
  never regresses, so it is safe on every upsert.

Adds test_upsert_mark_rejects_over_max and test_upsert_mark_completes_submission_and_batch.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
2026-07-02 21:35:24 +00:00
CC WorkerandClaude Opus 4.8 cce46305c9 FX-2: surface born-digital per-part marks from auto-map
The board grammar already parses per-part marks ([N marks] / "Total for Question
N is M marks"), but they were dropped building the first-pass template and the
row mapper hardcoded max_marks=0 — so every born-digital paper came back with
zero marks for a teacher to re-key by hand.

Thread the parsed mark through: bands.py derive_bands now carries each part's
`marks`, template.build passes it into part_bands, and _map_first_pass_to_rows
reads it via a defensive _safe_marks (non-negative int; unknown/None -> 0, so
image-only OCR — which has no marks yet — is unchanged). Containers still roll up
from parts. Adds test_auto_map_surfaces_born_digital_part_marks.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
2026-07-02 21:30:28 +00:00
CC WorkerandClaude Opus 4.8 41614b78ef FX-3: project image-only papers to Neo4j after auto-map
Only the born-digital fast path enqueued project_template_safe; the async OCR
job (_run_auto_map_job) — where image-only papers, R3's primary target, are
routed — never projected, so those papers never reached the graph after
auto-map. Project at the end of the async job too.

Adds test_auto_map_ocr_path_projects_to_neo4j.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
2026-07-02 21:22:54 +00:00
CC WorkerandClaude Opus 4.8 1671518ca8 FX-1: auto-map re-run must not PK-collide with confirmed ghosts
_refresh_ai_rows deleted only unconfirmed AI rows, then bulk-inserted freshly
generated rows keyed by a deterministic uuid5 of (template_id, semantic key).
A confirmed ghost keeps that id, so a re-run re-emitted it → primary-key
conflict that failed the whole insert batch (reachable: auto-map is blocked only
when marks exist, not when ghosts are confirmed).

Fix: after the delete, read the ids that survived (confirmed-AI + manual) and
skip re-inserting any freshly generated row whose id matches — preserving the
teacher's curated row and making the insert PK-safe.

Adds test_auto_map_rerun_after_confirm_does_not_pk_collide, which confirms a
ghost at its REAL generated id (the prior test used an arbitrary id that never
collided) and asserts the row survives exactly once with confirmed=True.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
2026-07-02 21:02:10 +00:00
6 changed files with 141 additions and 3 deletions
+4 -1
View File
@@ -67,11 +67,13 @@ def derive_bands(result, doc=None, rapid_glob=None):
topnum = _topnumber_boxes(docs) topnum = _topnumber_boxes(docs)
# gather parts with geometry, grouped by page # gather parts with geometry, grouped by page
by_page = defaultdict(list) # page -> [(q, label, t, b)] by_page = defaultdict(list) # page -> [(q, label, t, b)]
part_marks = {} # (question, part label) -> parsed marks (born-digital grammar)
for q in result.get("questions", []): for q in result.get("questions", []):
for p in q["parts"]: for p in q["parts"]:
bb, pg = p.get("bbox"), p.get("page") bb, pg = p.get("bbox"), p.get("page")
if bb and pg: if bb and pg:
by_page[pg].append((q["question"], p["label"], bb["t"], bb["b"])) by_page[pg].append((q["question"], p["label"], bb["t"], bb["b"]))
part_marks[(q["question"], p["label"])] = p.get("marks")
# global first page each question appears on (to mark the true start vs continuation pages) # global first page each question appears on (to mark the true start vs continuation pages)
q_first_page = {} q_first_page = {}
@@ -104,7 +106,8 @@ def derive_bands(result, doc=None, rapid_glob=None):
for (q, lab), st, en, _ in _ends(part_items): for (q, lab), st, en, _ in _ends(part_items):
qen = main_band.get(q, (st, 0))[1] # don't run past the question end qen = main_band.get(q, (st, 0))[1] # don't run past the question end
part.append({"label": lab, "question": q, part.append({"label": lab, "question": q,
"y_start": round(st, 1), "y_end": round(max(en, qen), 1)}) "y_start": round(st, 1), "y_end": round(max(en, qen), 1),
"marks": part_marks.get((q, lab))})
pages[pg] = {"main": main, "part": part} pages[pg] = {"main": main, "part": part}
return {"board": result.get("board"), "paper_code": result.get("paper_code"), return {"board": result.get("board"), "paper_code": result.get("paper_code"),
+1
View File
@@ -159,6 +159,7 @@ def build(structured, bands, furniture, pdf=None, page_roles=None):
"y_start": p["y_start"], "y_end": p["y_end"], "y_start": p["y_start"], "y_end": p["y_end"],
"label_box": part_bbox.get(p["label"]), # anchor, not the part extent "label_box": part_bbox.get(p["label"]), # anchor, not the part extent
"box": synthesize_part_box(p, xband), "box": synthesize_part_box(p, xband),
"marks": p.get("marks"), # parsed per-part marks (born-digital)
"source": "auto", "confirmed": False, "source": "auto", "confirmed": False,
}) })
pr = page_roles.get(pgs) or page_roles.get(pg) or {} pr = page_roles.get(pgs) or page_roles.get(pg) or {}
+42
View File
@@ -245,6 +245,36 @@ async def batch_csv(
# ─── marks ─────────────────────────────────────────────────────────────────── # ─── marks ───────────────────────────────────────────────────────────────────
def _advance_completion(ctx: ExamContext, batch_id: str, submission_id: str) -> None:
"""After a mark upsert, advance statuses: a submission with a mark for every markable (leaf)
question → complete; a batch whose every non-absent submission is complete → complete. Nothing
here regresses a status (only promotes to complete), so it is safe to run on every upsert."""
batch = _first(
ctx.supabase.table("marking_batches").select("id, template_id, status").eq("id", batch_id).limit(1).execute()
)
if not batch:
return
markable = {
q["id"] for q in _rows(
ctx.supabase.table("exam_questions").select("id, is_container").eq("template_id", batch["template_id"]).execute()
) if not q.get("is_container")
}
if not markable:
return
marked = {
m["question_id"] for m in _rows(
ctx.supabase.table("mark_entries").select("question_id").eq("submission_id", submission_id).execute()
)
}
if not markable.issubset(marked):
return
ctx.supabase.table("student_submissions").update({"status": "complete"}).eq("id", submission_id).execute()
subs = _rows(ctx.supabase.table("student_submissions").select("status").eq("batch_id", batch_id).execute())
active = [s for s in subs if s.get("status") != "absent"]
if active and all(s.get("status") == "complete" for s in active) and batch.get("status") != "complete":
ctx.supabase.table("marking_batches").update({"status": "complete"}).eq("id", batch_id).execute()
@router.put("/marks/{mark_id}") @router.put("/marks/{mark_id}")
async def upsert_mark( async def upsert_mark(
mark_id: str, mark_id: str,
@@ -259,6 +289,15 @@ async def upsert_mark(
if not submission: if not submission:
raise HTTPException(status_code=404, detail="Submission not found") raise HTTPException(status_code=404, detail="Submission not found")
# Reject an award that exceeds the question's max (only when a max is actually set; 0/None means
# "not scored yet" for AI/unmapped questions, so we can't validate those).
question = _first(
ctx.supabase.table("exam_questions").select("id, max_marks").eq("id", body.question_id).limit(1).execute()
)
max_marks = (question or {}).get("max_marks")
if isinstance(max_marks, (int, float)) and max_marks > 0 and body.awarded_marks is not None and body.awarded_marks > max_marks:
raise HTTPException(status_code=422, detail=f"awarded_marks {body.awarded_marks} exceeds max_marks {max_marks} for this question")
row = { row = {
"id": mark_id, "id": mark_id,
"submission_id": body.submission_id, "submission_id": body.submission_id,
@@ -285,6 +324,9 @@ async def upsert_mark(
if submission.get("status") in ("absent", "unmatched"): if submission.get("status") in ("absent", "unmatched"):
ctx.supabase.table("student_submissions").update({"status": "marking"}).eq("id", body.submission_id).execute() ctx.supabase.table("student_submissions").update({"status": "marking"}).eq("id", body.submission_id).execute()
# Promote the submission/batch to complete once every markable question has a mark.
_advance_completion(ctx, submission["batch_id"], body.submission_id)
return upserted return upserted
+22 -2
View File
@@ -465,6 +465,17 @@ def _safe_confidence(value: Any = None) -> float:
return 0.75 return 0.75
def _safe_marks(value: Any = None) -> int:
"""Parsed per-part marks → a non-negative int; unknown/None → 0 (image-only OCR has no marks yet)."""
if isinstance(value, bool):
return 0
if isinstance(value, (int, float)):
return max(0, int(value))
if isinstance(value, str) and value.strip().isdigit():
return int(value.strip())
return 0
def _margin_values(first_pass: Dict[str, Any], page_number: int) -> Dict[str, Optional[float]]: def _margin_values(first_pass: Dict[str, Any], page_number: int) -> Dict[str, Optional[float]]:
vals: Dict[str, Optional[float]] = {"left": None, "right": None, "top": None, "bottom": None} vals: Dict[str, Optional[float]] = {"left": None, "right": None, "top": None, "bottom": None}
for m in first_pass.get("margins") or []: for m in first_pass.get("margins") or []:
@@ -541,7 +552,7 @@ def _map_first_pass_to_rows(template_id: str, first_pass: Dict[str, Any], pdf_by
top = max(float(y1), float(y2)); bottom = min(float(y1), float(y2)) top = max(float(y1), float(y2)); bottom = min(float(y1), float(y2))
bounds = _box_to_canvas({"l": margins["left"], "r": margins["right"], "t": top, "b": bottom, "coord_origin": "BOTTOMLEFT"}, page_number, pages_geom) bounds = _box_to_canvas({"l": margins["left"], "r": margins["right"], "t": top, "b": bottom, "coord_origin": "BOTTOMLEFT"}, page_number, pages_geom)
bounds = bounds or _box_to_canvas(band.get("label_box"), page_number, pages_geom) bounds = bounds or _box_to_canvas(band.get("label_box"), page_number, pages_geom)
questions.append({"id": pid, "template_id": template_id, "parent_id": parent_id, "label": label, "order": len(questions), "max_marks": 0, "is_container": False, "bounds": bounds, "page": page_number, "source": "ai", "confirmed": False, "confidence": _safe_confidence(band.get("confidence")), "derivation": "docling-part-band-x-margins"}) questions.append({"id": pid, "template_id": template_id, "parent_id": parent_id, "label": label, "order": len(questions), "max_marks": _safe_marks(band.get("marks")), "is_container": False, "bounds": bounds, "page": page_number, "source": "ai", "confirmed": False, "confidence": _safe_confidence(band.get("confidence")), "derivation": "docling-part-band-x-margins"})
default_qid = questions[0]["id"] if questions else _ai_id(template_id, "question", "auto") default_qid = questions[0]["id"] if questions else _ai_id(template_id, "question", "auto")
for page_key in sorted(pages_obj, key=lambda k: int(k)): for page_key in sorted(pages_obj, key=lambda k: int(k)):
@@ -602,7 +613,13 @@ def _refresh_ai_rows(ctx: ExamContext, template_id: str, rows: Dict[str, List[Di
for table in ("exam_response_areas", "exam_boundaries", "exam_template_layout", "exam_questions"): for table in ("exam_response_areas", "exam_boundaries", "exam_template_layout", "exam_questions"):
sb.table(table).delete().eq("template_id", template_id).eq("source", "ai").eq("confirmed", False).execute() sb.table(table).delete().eq("template_id", template_id).eq("source", "ai").eq("confirmed", False).execute()
for table, key in (("exam_questions", "questions"), ("exam_response_areas", "response_areas"), ("exam_boundaries", "boundaries"), ("exam_template_layout", "layout")): for table, key in (("exam_questions", "questions"), ("exam_response_areas", "response_areas"), ("exam_boundaries", "boundaries"), ("exam_template_layout", "layout")):
payload = _dedupe_rows_by_id(rows.get(key) or []) # A row that survived the delete above is confirmed-AI or manual — the teacher's curated version.
# AI ids are a deterministic uuid5 of (template_id, semantic key), so a re-run re-emits the SAME id
# for a confirmed ghost; re-inserting it would PK-collide and fail the whole batch. Skip those ids so
# confirmed work is preserved and the insert is safe (fixes the re-map-after-confirm collision).
kept = sb.table(table).select("id").eq("template_id", template_id).execute()
kept_ids = {str(r["id"]) for r in (getattr(kept, "data", None) or []) if isinstance(r, dict) and r.get("id")}
payload = [r for r in _dedupe_rows_by_id(rows.get(key) or []) if str(r.get("id")) not in kept_ids]
if payload: if payload:
sb.table(table).insert(payload).execute() sb.table(table).insert(payload).execute()
@@ -634,6 +651,9 @@ def _run_auto_map_job(job_id: str, ctx: ExamContext, template_id: str, pdf_bytes
_set_auto_map_status(job_id, {"status": "running", "template_id": template_id}) _set_auto_map_status(job_id, {"status": "running", "template_id": template_id})
try: try:
rows = _run_auto_map_merge(ctx, template_id, pdf_bytes, source_label) rows = _run_auto_map_merge(ctx, template_id, pdf_bytes, source_label)
# Project to Neo4j like the born-digital fast path does — otherwise image-only papers (R3's
# primary target, routed here because they need OCR) never reach the graph after auto-map.
project_template_safe(template_id)
_set_auto_map_status(job_id, {"status": "completed", "template_id": template_id, "counts": {k: len(v) for k, v in rows.items()}}) _set_auto_map_status(job_id, {"status": "completed", "template_id": template_id, "counts": {k: len(v) for k, v in rows.items()}})
except Exception as exc: except Exception as exc:
logger.exception(f"auto-map job failed for template {template_id}: {exc}") logger.exception(f"auto-map job failed for template {template_id}: {exc}")
+24
View File
@@ -249,6 +249,30 @@ def test_upsert_mark_submission_404():
assert c.put("/api/exam/marks/mk-x", json={"submission_id": "nope", "question_id": "q1", "awarded_marks": 1}).status_code == 404 assert c.put("/api/exam/marks/mk-x", json={"submission_id": "nope", "question_id": "q1", "awarded_marks": 1}).status_code == 404
def test_upsert_mark_rejects_over_max():
c = make_client(_batch_with_cohort()) # q1 max_marks = 3
r = c.put("/api/exam/marks/mk-over", json={"submission_id": "sub2", "question_id": "q1", "awarded_marks": 4})
assert r.status_code == 422
def test_upsert_mark_completes_submission_and_batch():
store = base_store(
marking_batches=[{"id": "b1", "template_id": TPL, "institute_id": INST_A, "teacher_id": TEACHER, "status": "marking"}],
exam_questions=[
{"id": "q0", "template_id": TPL, "label": "Q1", "max_marks": 0, "order": 0, "is_container": True},
{"id": "q1", "template_id": TPL, "label": "01", "max_marks": 3, "order": 1, "is_container": False},
{"id": "q2", "template_id": TPL, "label": "02", "max_marks": 5, "order": 2, "is_container": False},
],
student_submissions=[{"id": "sub1", "batch_id": "b1", "student_id": "s1", "status": "marking"}],
mark_entries=[{"id": "m1", "batch_id": "b1", "submission_id": "sub1", "question_id": "q1", "awarded_marks": 2}],
)
c = make_client(store)
# marking the last leaf question completes the submission (container q0 doesn't block) and the batch
assert c.put("/api/exam/marks/m2", json={"submission_id": "sub1", "question_id": "q2", "awarded_marks": 4}).status_code == 200
assert next(s for s in store["student_submissions"] if s["id"] == "sub1")["status"] == "complete"
assert next(b for b in store["marking_batches"] if b["id"] == "b1")["status"] == "complete"
# ─── scans (E3 guards) ─────────────────────────────────────────────────────── # ─── scans (E3 guards) ───────────────────────────────────────────────────────
def _batch_store(): def _batch_store():
+48
View File
@@ -642,6 +642,21 @@ def test_auto_map_fast_path_merges_ai_rows_and_returns_detail(monkeypatch):
assert store["exam_boundaries"] and store["exam_boundaries"][0]["derivation"] == "docling-main-band" assert store["exam_boundaries"] and store["exam_boundaries"][0]["derivation"] == "docling-main-band"
def test_auto_map_surfaces_born_digital_part_marks(monkeypatch):
# Regression: the born-digital grammar parses per-part marks, but the row mapper hardcoded
# max_marks=0. A part band carrying `marks` must flow through to the question row's max_marks.
store = _template_with_source()
store.update({"exam_questions": [], "exam_response_areas": [], "exam_boundaries": [], "exam_template_layout": []})
client, store = make_client(store=store)
fp = _first_pass_template()
fp["pages"]["1"]["part_bands"][0]["marks"] = 4
_patch_auto_map(monkeypatch, store, fast=True)
monkeypatch.setattr(templates_mod, "auto_map", lambda *_a, **_k: fp) # override with the marked part band
assert client.post("/api/exam/templates/t1/auto-map").status_code == 200
part = next(q for q in store["exam_questions"] if q.get("label") == "01.1")
assert part["max_marks"] == 4
def test_auto_map_deduplicates_repeated_response_area_ids(monkeypatch): def test_auto_map_deduplicates_repeated_response_area_ids(monkeypatch):
store = _template_with_source() store = _template_with_source()
client, store = make_client(store=store) client, store = make_client(store=store)
@@ -674,6 +689,27 @@ def test_auto_map_preserves_manual_and_confirmed_rows_on_rerun(monkeypatch):
assert "old-ai" not in ids assert "old-ai" not in ids
def test_auto_map_rerun_after_confirm_does_not_pk_collide(monkeypatch):
# Regression: a confirmed ghost keeps its DETERMINISTIC uuid5 id, which auto-map re-emits on a
# re-run — the old code re-inserted that id and PK-collided. Use the real generated id (not an
# arbitrary one, unlike the test above) so the collision path is actually exercised.
store = _template_with_source()
store.update({"exam_questions": [], "exam_response_areas": [], "exam_boundaries": [], "exam_template_layout": []})
client, store = make_client(store=store)
_patch_auto_map(monkeypatch, store, fast=True)
assert client.post("/api/exam/templates/t1/auto-map").status_code == 200
ghost_id = next(q["id"] for q in store["exam_questions"] if q["source"] == "ai")
# teacher confirms that ghost (row kept, deterministic id unchanged)
for q in store["exam_questions"]:
if q["id"] == ghost_id:
q["confirmed"] = True
# re-run auto-map: must not re-insert the same id, and must preserve the teacher's confirmation
assert client.post("/api/exam/templates/t1/auto-map").status_code == 200
same = [r for r in store["exam_questions"] if r["id"] == ghost_id]
assert len(same) == 1, "confirmed ghost must not be duplicated (PK collision) on re-run"
assert same[0]["confirmed"] is True, "teacher's confirmation must survive a re-run"
def test_auto_map_non_owner_is_403_before_download(monkeypatch): def test_auto_map_non_owner_is_403_before_download(monkeypatch):
store = _template_with_source(owner=OTHER_TEACHER) store = _template_with_source(owner=OTHER_TEACHER)
client, store = make_client(user_id=TEACHER, institute_ids=(INST_A,), store=store) client, store = make_client(user_id=TEACHER, institute_ids=(INST_A,), store=store)
@@ -718,4 +754,16 @@ def test_auto_map_ocr_returns_job_id_and_status_completes(monkeypatch):
body = status.json() body = status.json()
assert body["status"] == "completed" assert body["status"] == "completed"
assert body["counts"]["questions"] >= 2 assert body["counts"]["questions"] >= 2
def test_auto_map_ocr_path_projects_to_neo4j(monkeypatch, _stub_projection):
# Image-only papers route through the async OCR job; regression: that path must project to Neo4j
# like the born-digital fast path, or the graph is never built for the primary target.
store = _template_with_source()
client, store = make_client(store=store)
_patch_auto_map(monkeypatch, store, fast=False)
resp = client.post("/api/exam/templates/t1/auto-map")
assert resp.status_code == 202
# the BackgroundTask runs after the response under TestClient
assert "t1" in _stub_projection
assert body["template"]["layout"] assert body["template"]["layout"]