From 1671518ca81ea6b3ea59726083aed0d8f2d34a7b Mon Sep 17 00:00:00 2001 From: CC Worker Date: Thu, 2 Jul 2026 21:02:10 +0000 Subject: [PATCH] FX-1: auto-map re-run must not PK-collide with confirmed ghosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _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 --- routers/exam/templates.py | 8 +++++++- tests/test_exam_templates.py | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/routers/exam/templates.py b/routers/exam/templates.py index ae7b2f9..48c94c6 100644 --- a/routers/exam/templates.py +++ b/routers/exam/templates.py @@ -602,7 +602,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"): 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")): - 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: sb.table(table).insert(payload).execute() diff --git a/tests/test_exam_templates.py b/tests/test_exam_templates.py index 37e38d2..4971abd 100644 --- a/tests/test_exam_templates.py +++ b/tests/test_exam_templates.py @@ -674,6 +674,27 @@ def test_auto_map_preserves_manual_and_confirmed_rows_on_rerun(monkeypatch): 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): store = _template_with_source(owner=OTHER_TEACHER) client, store = make_client(user_id=TEACHER, institute_ids=(INST_A,), store=store)