From 66364e0e927a4134e37a9bb266ae459c34ad6bd6 Mon Sep 17 00:00:00 2001 From: Zareian Date: Tue, 22 Sep 2026 04:43:44 +0330 Subject: [PATCH] fix the campaigns concurrency test by fixing what it was catching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ExecuteCampaignRealConcurrencyTests had been written off as a known flake for weeks — about 45% red on a full-suite run. It was not timing noise. It was reporting a real defect, and the defect is in production code. execute_campaign looped with .iterator(), which keeps the read cursor on the user table open for the whole loop — a loop that writes a CampaignDelivery per user and, on the sms branch, makes a network call to Kavenegar in between. On SQLite that open cursor locks the table against any concurrent writer, and the lock is not released until the loop ends. So _retry_on_sqlite_lock could never recover from it no matter how long it waited: widening its window from 0.5s to 2s (also done here, since the linear backoff was too tight regardless) moved the failure rate not at all on its own. Both branches now materialise the ids before the first write. After the change: 20 consecutive runs of the test itself and 3 full-suite runs, all green. This is not only a test fix. While we are on SQLite the same pattern produces the same lock in production, where it surfaces as "database is locked" on unrelated requests that happen to land during a campaign run. Also in this pass ----------------- - CLAUDE.md carried three sections each claiming to be the current state, with three different question counts (1,441 / 1,783 / reality 2,463). There is now one current-state section and the older two are labelled as history, so whoever picks this up next is not reading stale numbers as fact. Three other figures that had drifted are corrected against the database: the verdict split, the figure-gap share (881/~42% is now 508/~14% after the five books were re-read), and the node coverage. - Dropped four genuinely unused imports. The remaining pyflakes hits are the signal registrations in apps.py, which are imported for effect. - .env.example suggested gpt-4o for vision; gpt-4.1-mini measured both cheaper and more accurate and is what the server actually runs. Audited all 2,463 questions across every grade: no question with the wrong number of options, none with zero or two correct answers, no empty or duplicated option, no orphan lesson or node, no duplicate question inside a lesson. 458 tests pass. Co-Authored-By: Claude Opus 5 --- .env.example | 2 +- CLAUDE.md | 58 ++++++++++++++++--- campaigns/services.py | 55 ++++++++++++------ config/tests.py | 1 - .../generate_questions_from_chunks.py | 1 - content/tests_question_io.py | 2 +- rag/management/commands/rag_ingest_book.py | 2 +- 7 files changed, 89 insertions(+), 32 deletions(-) diff --git a/.env.example b/.env.example index df99e99..ca8466a 100644 --- a/.env.example +++ b/.env.example @@ -74,7 +74,7 @@ OPENAI_BASE_URL=https://api.gapgpt.app/v1 # OPENAI_BASE_URL=https://api.openai.com/v1 OPENAI_API_KEY_OPENAI_DIRECT= OPENAI_CHAT_MODEL=gpt-4o-mini -OPENAI_VISION_MODEL=gpt-4o +OPENAI_VISION_MODEL=gpt-4.1-mini # این رو تغییر نده؛ باید با مدلی که تیمِ دیگه باهاش collectionها رو ساخته یکی باشه. OPENAI_EMBEDDING_MODEL=text-embedding-3-small diff --git a/CLAUDE.md b/CLAUDE.md index 7f26625..a463735 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -191,8 +191,10 @@ problem. These are invisible in retrieval and only surface when something trusts `rag_verify_solutions` re-solves every extracted problem independently and compares. `LessonChunk.solution_verdict` is three-state — `agrees` / `disagrees` / `unjudgeable` — and the verdict also rides in the Qdrant payload under a key `retrieval._payload_text` -does not read, so the tutor's context is unchanged. Current state: 1,112 agree, 123 -disagree, 881 unjudgeable. **Question generation only ever reads `agrees` chunks.** +does not read, so the tutor's context is unchanged. Current state (2026-09-22, across +all 3,517 chunks): 2,466 agree, 354 disagree, 508 unjudgeable, 189 not yet checked — +those 189 are the other team's علوم/مطالعات collections, never run through the checker. +**Question generation only ever reads `agrees` chunks.** Two things that were learned the expensive way and are already encoded in the prompts: @@ -205,11 +207,13 @@ independent answer is stored separately in `checked_answer`. ## The figure gap (largest open data issue) -881 chunks (~42%) reference a figure or table that is not in the text, so they cannot be +508 chunks (~14%) reference a figure or table that is not in the text, so they cannot be answered or turned into questions. **The vision model saw the figure; it was simply never asked to write it down.** `rag/services/pdf.py` now carries a self-contained rule telling -it to inline measurements and labels into the problem text — books ingested before that -rule predate it. Recovering them means re-ingesting those pages (~$2.7 of API credit). +it to inline measurements and labels into the problem text, and re-reading the five math +books under that rule is what brought this down from 881 (~42%). What is left is +concentrated in the three books that predate the rule (سوم/ششم/هفتم, the other team's); +recovering them means re-ingesting those pages (~$2.7 of API credit). ## Quiz assembly @@ -218,7 +222,8 @@ and a lesson here is the finest grain — one problem type from the book, not a session. `GET /api/nodes/{id}/quiz` assembles a quiz from a node's lessons, at most one question per lesson. Shuffle is seeded on `nodeId:userId`, so a refresh returns the same quiz; `questionCount` is reported separately from `requested` so a thin node cannot look -like an error. 172 of 230 nodes currently hold a full four-question quiz. +like an error. 224 of 263 nodes hold a full four-question quiz, and every node in +grades 8–12 does; the 39 that fall short are all in grades 3/6/7. ## AI provider notes @@ -348,7 +353,44 @@ preferred setup, because there is then no key to leak. **Never let the server ru `django-insecure-…` fallback from `config/settings.py`: that string is committed here, so anyone who can read this repo can forge a token for any user.** -## State as of 2026-09-17 (evening) — read this first +## وضعیت فعلی (۲۰۲۶-۰۹-۲۲) — این را اول بخوان + +سرور روی `api.math.tadrisino2.ir` زنده است و **کدِ فعلی را سرو می‌کند** (سرویس واقعاً +ری‌استارت شد، نه فقط فایل‌ها روی دیسک). `/api/health` می‌دهد: +`grades: 8, lessons: 2617, questions: 2463` و هیچ مهاجرتِ اعمال‌نشده‌ای ندارد. + +- **هر نودِ پایه‌های هشتم تا دوازدهم آزمونِ کاملِ چهارسوالی دارد.** پایه‌های + سوم/ششم/هفتم کتابِ تیمِ دیگرند: ۳۹ نود از ۱۴۵ کمتر از چهار سوال دارند، ولی هیچ‌کدام + خالی نیست، پس روی نقشه جزیره‌ی مرده وجود ندارد. +- ۴۵۸ تست سبز، `check` و `check --deploy` و `makemigrations --check` تمیز، و + `spectacular` بدونِ خطا و هشدار (قرارداد کامل است). +- تستِ هم‌زمانیِ campaigns که مدت‌ها flaky بود **دیگر flaky نیست** — علتش یک باگِ + واقعی بود، نه طبیعتِ تست؛ پایینِ همین فایل توضیح داده شده. + +بخش‌های تاریخیِ زیر به‌ترتیبِ زمان نگه داشته شده‌اند چون درس‌هایشان هنوز معتبر است؛ +**اعدادِ محتوایی‌شان کهنه‌اند و ملاک همین بخش است.** + +--- + +### تستِ flakyِ campaigns یک باگِ واقعی بود (۲۰۲۶-۰۹-۲۲) + +`ExecuteCampaignRealConcurrencyTests` ماه‌ها به‌عنوانِ «flakyِ شناخته‌شده» کنار گذاشته +شده بود (~۴۵٪ قرمز روی اجرای کاملِ سوئیت). علتش تصادفی‌بودنِ زمان‌بندی نبود: + +`execute_campaign` با `.iterator()` روی کاربرها حلقه می‌زد، یعنی **کرسرِ خواندن در +تمامِ طولِ حلقه باز می‌ماند** — حلقه‌ای که وسطش `CampaignDelivery` می‌نویسد و در شاخه‌ی +sms یک فراخوانیِ شبکه‌ای به کاوه‌نگار هم می‌کند. روی SQLite همین کرسرِ باز جدول را برای +هر نویسنده‌ی هم‌زمان قفل می‌کند، و قفل تا پایانِ حلقه رها نمی‌شود — پس +`_retry_on_sqlite_lock` هیچ‌وقت نمی‌توانست نجاتش دهد، هر چقدر هم صبر می‌کرد. + +حالا هر دو شاخه پیش از اولین نوشتن، نتیجه را با `list(...values_list(...))` مادی +می‌کنند. بعد از این تغییر: ۲۰ اجرای پیاپیِ خودِ تست و ۳ اجرای کاملِ سوئیت، همه سبز. + +⚠️ **درسِ کلی: در این کدبیس `.iterator()` را دورِ حلقه‌ای که می‌نویسد یا I/O بیرونی +می‌زند نگذارید.** تا وقتی روی SQLite هستیم، این الگو در پروداکشن هم همان قفل را +می‌سازد — آنجا به‌شکلِ «database is locked» برای درخواست‌های بی‌ربطِ دیگر. + +## تاریخچه — وضعیت ۲۰۲۶-۰۹-۱۷ (اعداد کهنه) **The server is live and running current code.** `main` is at the latest commit, the service `gunicorn-mathapp` was restarted against it, and `/api/health` reports @@ -389,7 +431,7 @@ The frontend's `Question` type carries a `type` field that nothing reads, and `count_objects` in `Question.TYPE_CHOICES` is an empty shell — no rows, no serializer branch, no rendering anywhere. Do not build content for it without asking first. -## وضعیت ۱۴۰۵/۰۶/۲۹ (۲۰۲۶-۰۹-۲۰) — این را اول بخوان +## تاریخچه — وضعیت ۱۴۰۵/۰۶/۲۹ (۲۰۲۶-۰۹-۲۰) (اعداد کهنه) سرور روی `3026df0` است، کد و محتوا هر دو منتشر شدند، سرویس واقعاً ری‌استارت شد، و `/api/health` عدد `questions: 1783` می‌دهد. `/api/nodes/1/quiz` چهار سوالِ واقعی برمی‌گرداند. diff --git a/campaigns/services.py b/campaigns/services.py index ac1cdb7..a6f9d3f 100644 --- a/campaigns/services.py +++ b/campaigns/services.py @@ -20,16 +20,20 @@ from notifications.services import notify from .models import CampaignDelivery, MessageCampaign -def _retry_on_sqlite_lock(fn, *, retries=5, base_delay=0.05): +def _retry_on_sqlite_lock(fn, *, retries=8, base_delay=0.05, max_delay=0.4): """ - روی SQLite (مخصوصاً دیتابیسِ حافظه‌ایِ shared-cache که تست‌ها ازش - استفاده می‌کنن)، وقتی دو اتصالِ هم‌زمان می‌خوان همون جدول رو بنویسن، - ممکنه OperationalError('database table is locked') بگیریم. این - SQLITE_LOCKED‌ه نه SQLITE_BUSY، پس busy_timeoutِ معمولیِ اتصال کمکی - نمی‌کنه و بلافاصله raise می‌شه؛ یک خطای واقعیِ داده نیست، فقط - تناقضِ لحظه‌ایِ لاکه. این تابع فقط همین خطای گذرا رو با چند بار - تلاشِ سریع (backoffِ کوچیکِ فزاینده) دوباره امتحان می‌کنه؛ هر + روی SQLite وقتی دو اتصالِ هم‌زمان می‌خوان همون جدول رو بنویسن، ممکنه + OperationalError('database table is locked') بگیریم. این SQLITE_LOCKED‌ه + نه SQLITE_BUSY، پس busy_timeoutِ اتصال کمکی نمی‌کنه و بلافاصله raise + می‌شه؛ خطای واقعیِ داده نیست، فقط تناقضِ لحظه‌ایِ لاک. هر OperationalError دیگه (یا شکستِ تلاشِ آخر) دست‌نخورده بالا می‌ره. + + ⚠️ پنجره‌ی انتظار باید از خودِ قفل درازتر باشد، وگرنه محافظ فقط *گاهی* + کار می‌کند. نسخه‌ی اول ۵ تلاش با backoffِ خطی داشت — رویِ‌هم ۰٫۵ ثانیه — + و تستِ هم‌زمانیِ campaigns را در حدودِ نیمی از اجراهای کاملِ سوئیت قرمز + می‌کرد، چون زیرِ بارِ کلِ تست‌ها قفل بیشتر از آن طول می‌کشد. حالا + backoff نمایی است و تا ~۲ ثانیه صبر می‌کند؛ سقفِ max_delay هم هست تا + یک قفلِ طولانی، درخواستِ کاربر را بی‌نهایت معلق نگه ندارد. """ for attempt in range(retries): try: @@ -37,7 +41,7 @@ def _retry_on_sqlite_lock(fn, *, retries=5, base_delay=0.05): except OperationalError as exc: if 'locked' not in str(exc).lower() or attempt == retries - 1: raise - time.sleep(base_delay * (attempt + 1)) + time.sleep(min(base_delay * (2 ** attempt), max_delay)) # عمداً به notifications.models.Notification.TYPE_CHOICES اضافه نشده: آن # choices فقط رویدادهاییه که notifications به‌عنوانِ listener گوش می‌ده، @@ -220,16 +224,26 @@ def execute_campaign(campaign: MessageCampaign) -> None: already_sent_user_ids = _already_sent_user_ids(campaign) if campaign.channel == 'in_app': - for user in users.only('id').iterator(): - if user.id in already_sent_user_ids: + # ⚠️ عمداً list() و نه .iterator(): کرسرِ باز باید *پیش از* اولین + # نوشتن بسته شود. با .iterator() خواندنِ کاربرها در تمامِ طولِ حلقه + # باز می‌ماند و روی SQLite همان، جدول را برای هر نویسنده‌ی هم‌زمان + # قفل می‌کند — قفلی که تا پایانِ حلقه رها نمی‌شود، پس هیچ مقدار + # retry هم نجاتش نمی‌دهد. این دقیقاً همان چیزی بود که تستِ + # هم‌زمانیِ campaigns را در حدودِ نیمی از اجراها قرمز می‌کرد. + # + # حافظه: اینجا فقط idها نگه داشته می‌شوند و این ماژول طبقِ + # docstringِ بالا برای لیست‌های کوچک است، نه کمپینِ انبوه. + for user_id in list(users.values_list('id', flat=True)): + if user_id in already_sent_user_ids: continue - delivery = _claim_delivery(campaign, user.id) + delivery = _claim_delivery(campaign, user_id) if delivery is None: continue - def _notify_and_mark_sent(): + + def _notify_and_mark_sent(user_id=user_id, delivery=delivery): with transaction.atomic(): notify( - user_id=user.id, + user_id=user_id, type=CAMPAIGN_NOTIFICATION_TYPE, title=campaign.title, body=campaign.body, @@ -251,16 +265,19 @@ def execute_campaign(campaign: MessageCampaign) -> None: return if campaign.channel == 'sms': - for user in users.only('id', 'phone').iterator(): - if user.id in already_sent_user_ids: + # همان دلیلِ شاخه‌ی in_app بالا، و اینجا حتی مهم‌تر: وسطِ این حلقه + # یک فراخوانیِ شبکه‌ای به کاوه‌نگار انجام می‌شود، پس کرسرِ باز + # می‌توانست به‌اندازه‌ی کلِ ارسالِ پیامک‌ها جدول را قفل نگه دارد. + for user_id, phone in list(users.values_list('id', 'phone')): + if user_id in already_sent_user_ids: continue - if not user.phone: + if not phone: continue - delivery = _claim_delivery(campaign, user.id) + delivery = _claim_delivery(campaign, user_id) if delivery is None: continue try: - account_services.send_sms(phone=user.phone, message=campaign.body) + account_services.send_sms(phone=phone, message=campaign.body) except SMSDeliveryError as exc: _mark_delivery_result(delivery, error=exc) else: diff --git a/config/tests.py b/config/tests.py index 9f3aaae..083a296 100644 --- a/config/tests.py +++ b/config/tests.py @@ -5,7 +5,6 @@ import os from unittest.mock import patch from django.conf import settings -from django.core.exceptions import ImproperlyConfigured from django.test import RequestFactory, SimpleTestCase, TestCase from rest_framework.test import APIClient diff --git a/content/management/commands/generate_questions_from_chunks.py b/content/management/commands/generate_questions_from_chunks.py index 7f04069..ad19b06 100644 --- a/content/management/commands/generate_questions_from_chunks.py +++ b/content/management/commands/generate_questions_from_chunks.py @@ -27,7 +27,6 @@ import threading import time from concurrent.futures import ThreadPoolExecutor -from django.conf import settings from django.core.management.base import BaseCommand, CommandError from django.db import transaction diff --git a/content/tests_question_io.py b/content/tests_question_io.py index 591bf43..0df9252 100644 --- a/content/tests_question_io.py +++ b/content/tests_question_io.py @@ -8,7 +8,7 @@ import json from django.test import TestCase -from content.models import Choice, Grade, Lesson, Node, Question, Unit +from content.models import Grade, Lesson, Node, Question, Unit from content.question_io import ImportError_, export_questions, import_questions, parse diff --git a/rag/management/commands/rag_ingest_book.py b/rag/management/commands/rag_ingest_book.py index 5e06768..790e990 100644 --- a/rag/management/commands/rag_ingest_book.py +++ b/rag/management/commands/rag_ingest_book.py @@ -16,7 +16,7 @@ from rag.services.config import build_collection_name, rag_is_configured from rag.services.ingest import (_done_pages, estimate_page_cost, ingest_book, pages_needing_refresh) from rag.services.textbooks import TEXTBOOKS, get_textbook -from rag.models import IngestedPage, LessonChunk +from rag.models import LessonChunk class Command(BaseCommand):