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 <noreply@anthropic.com>
This commit is contained in:
parent
6c73174829
commit
1671518ca8
@ -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()
|
||||
|
||||
|
||||
@ -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)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user