docs: ADR 0004 competency mastery concurrency

docs/openedx_learning/decisions/0002-competency-criteria-model.rst

@@ -240,9 +240,9 @@ Decision
    3. ``oel_tagging_objecttag(object_id)``
    4. ``CompetencyCriteria(oel_tagging_objecttag_id)``
    5. ``CompetencyCriteria(competency_criteria_group_id)``
-   6. ``StudentCompetencyCriteriaStatus(user_id, competency_criteria_id)``
-   7. ``StudentCompetencyCriteriaGroupStatus(user_id, competency_criteria_group_id)``
-   8. ``StudentCompetencyStatus(user_id, oel_tagging_tag_id)``
+   6. ``StudentCompetencyCriteriaStatus(user_id, competency_criteria_id)`` (unique)
+   7. ``StudentCompetencyCriteriaGroupStatus(user_id, competency_criteria_group_id)`` (unique)
+   8. ``StudentCompetencyStatus(user_id, oel_tagging_tag_id)`` (unique)
    9. ``CompetencyRuleProfile(scope_code)`` (unique -- at most one profile per distinct scope value; a plain unique constraint on the three raw nullable scope columns would not enforce this, since SQL never treats two ``NULL`` values as equal and this project's MySQL backend does not support the conditional/partial unique indexes that would otherwise route around that; see the ``scope_code`` column in Decision 3)
    10. ``CompetencyMasteryStatuses(status)`` (unique)
 
@@ -422,3 +422,11 @@ Rejected Alternatives
 
       1. Silently does not work on this project's tested and production database backend. Django compiles a conditional ``UniqueConstraint`` to a partial index, which MySQL does not support; Django raises only a non-fatal system-check warning (``models.W036``) and skips creating the constraint, leaving the uniqueness rule completely unenforced at the database level.
       2. The gap would surface only as a data-integrity incident under concurrent writes, not as a test or migration failure, since SQLite (used for quick local test runs) does support partial indexes and would mask the problem in that environment.
+
+Changelog
+---------
+
+2026-07-27:
+
+* Made the learner status indexes unique, so there is one row per learner and node. This is what
+  the in-place, monotone status updates in :ref:`openedx-learning-adr-0004` read, lock, and update.

docs/openedx_learning/decisions/0003-competency-criteria-versioning.rst

@@ -44,12 +44,18 @@ For the initial implementation, versioning and traceability of competency achiev
    - A ``CompetencyRuleProfile`` is "in use" if any ``CompetencyCriterion`` assigned to it (``competency_rule_profile_id``) has an associated ``StudentCompetencyCriteriaStatus`` row. Editing an in-use profile's ``rule_type``/``rule_payload`` requires the same warning and confirmation.
    - The same warning applies when creating a more specific profile causes existing criteria to be reassigned to it, and when an authoring action switches a criterion between a profile assignment and per-criterion overrides (ADR 0002 Decision 4).
 
-5. Learner status models/tables are append-only history and do not use ``django-simple-history``:
+5. Learner status models/tables are updated in-place:
 
-   - For ``StudentCompetencyCriteriaStatus``, ``StudentCompetencyCriteriaGroupStatus``, and ``StudentCompetencyStatus``, each status change is stored as a new row with ``created`` as the write timestamp.
-   - Existing learner status rows are not updated in place.
-   - Current status is determined by the most recent row for a given learner + target entity (ordered by ``created``, with ``id`` as a tie-breaker).
-   - Older rows represent the learner status history and remain available for audit/tracing.
+   - For ``StudentCompetencyCriteriaStatus``, ``StudentCompetencyCriteriaGroupStatus``, and ``StudentCompetencyStatus``,
+     each status change updates the responsible row.
+   - Automatic status updates only ever increase a status, as relied on by
+     :ref:`openedx-learning-adr-0004`. A downward adjustment (for example ``Demonstrated`` to
+     ``PartiallyAttempted``) is never applied by a grade change or by a competency criteria rule
+     change.
+   - Direct edits by staff, through Django admin or as a deliberate instructor correction, are
+     exempt: they may set a status to any value, including a lower one, and the ancestors above the
+     edited node are recomputed to match.
+   - How learner status history is retained is not decided here.
 
 
 Rejected Alternatives
@@ -85,3 +91,13 @@ Rejected Alternatives
     - Cons:
         - Requires custom tooling to reconstruct past versions
         - Does not align with existing publishable versioning patterns
+
+Changelog
+---------
+
+2026-07-27:
+
+* Reworked Decision 5 for :ref:`openedx-learning-adr-0004`: learner status rows are now updated in
+  place, and automatic updates only ever increase a status, with direct staff edits exempt.
+  Previously append-only, with current status resolved as the most recent row. How status history is
+  retained is left undecided.

docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst

@@ -0,0 +1,165 @@
+.. _openedx-learning-adr-0004:
+
+4. How should learner competency mastery be recorded concurrently and at scale?
+================================================================================
+
+Status
+------
+Proposed.
+
+Context
+-------
+A learner's mastery of one competency is stored at three levels of the criteria tree: the graded
+leaf criterion, each criteria group above it, and the competency itself. There is one row per
+learner and node, updated in place (:ref:`openedx-learning-adr-0002`,
+:ref:`openedx-learning-adr-0003`). Each row holds one of three values, lowest to highest:
+``AttemptedNotDemonstrated``, ``PartiallyAttempted``, ``Demonstrated``.
+
+One grade change updates the leaf and then every row above it, for many learners at once. This ADR
+decides how those updates stay correct when two of them for the same learner overlap.
+
+The problem: a group requires both Assignment A and Assignment B, and celery tasks recomputing
+grades and competency statuses for this worker run at the same time. That is likely to happen
+when instructor actions trigger multiple subsection grading events.
+Each of the two writers sees its own assignment done and the other still outstanding,
+so both write "not demonstrated" for the group. Both are wrong, both have finished, and nothing is
+left to correct it.
+
+Two constraints shape the answer. First, **the grading task cannot be one transaction**: it reads
+MongoDB and memcached, queues further celery tasks, publishes events, and triggers database writes
+owned by four other Django apps. Wrapping all of that would roll back other apps' data and publish
+events for a grade that never commits. Second, **everything above the leaf is derived**: a group's
+value can always be recalculated from the leaves beneath it, so the leaf is the only row that is a
+direct consequence of the grade.
+
+That constraint is why the whole grading task can't be one transaction. A separate constraint is why
+even a transaction scoped only to the leaf and its rollup would still be wrong: two tasks finishing different
+assignments for the same group race on that group's row regardless of transaction size, since each transaction
+hides its writes from the other until it commits (Rejected Alternative 2).
+
+Decision
+--------
+
+1. **Write the leaf status in the same transaction as the grade. Nothing above it.**
+   The grading task calls one openedx-core function, which writes the leaf, so the grade and its
+   leaf commit or fail together.
+
+2. **Schedule rollup as a secondary celery task, asynchronously after the grade is recorded.**
+   This separation avoids direct contention between concurrent grade changes on shared parent nodes.
+   The secondary task is automatically retried by celery if it fails.
+
+3. **Rollup commits each level before reading the next, with no locks.**
+   A writer sees only committed data, so whichever writer reads a parent last sees all its children
+   at their final values and computes the correct result. This depends on Decision 4, which provides
+   a reconciliation rule.
+
+4. **Automatic updates may only raise a status, never lower it.**
+   Each write stores whichever is higher, the stored or the newly computed value, so concurrent
+   writers cannot overwrite each other. A writer reading stale data can only compute a value that
+   is too low, and too low is discarded. That is also what makes celery's repeated and out-of-order
+   delivery harmless.
+
+5. **Add a manually-invoked recovery mechanism.**
+   For example a management command or Django admin action forces roll-ups to recalculate for a given range, to recover from
+   operational failures, content tagging errors, or bugs in the roll-up code that celery's retry
+   won't catch.
+
+6. **Only a direct staff edit may lower a status.** A staff correction may set any value, and the
+   rows above it are recalculated and overwritten rather than merged. A later grade change can raise
+   what an edit lowered, but never lower what an edit raised.    The staff correction takes a row lock
+   on the ``StudentCompetencyCriteriaGroupStatus`` row for that learner and the competency's root
+   criteria group, the group with no parent; no other path takes a lock.
+
+
+
+Rejected Alternatives
+---------------------
+
+1. Lock each criteria group row before recalculating it.
+
+    - Pros:
+        - Correctness comes from making contending writers take turns, which is easier to prove than
+          an argument about the order of commits and reads.
+    - Cons:
+        - One grade change can affect several leaves of the same tree, so a writer can need several
+          locks at once, which introduces deadlocks that need their own detection and retry code.
+        - It puts a lock wait on every grade change. MySQL waits 50 seconds by default, inside a task
+          allowed 300 seconds in total.
+        - Correctness would depend on the isolation level, silently, and SQLite has no row locks, so
+          the test suite could not exercise it.
+
+2. Share one transaction between the grade and the whole roll-up, not just the leaf.
+
+    - Pros:
+        - The grade and every mastery row it touches would commit or fail together, so no roll-up
+          could ever be left unfinished and Decisions 2 and 5 would be unnecessary.
+    - Cons:
+        - The grading task cannot be wrapped in a transaction at all, for the reasons in the Context.
+        - Wrapping only the roll-up is worse than doing nothing: it hides each writer's changes from
+          the other until both have finished, which is the problem in the Context again, one level up
+          the tree and harder to diagnose.
+
+3. Take one lock on the learner's competency root-group status row, then recalculate the whole
+   subtree beneath it.
+
+    - Pros:
+        - Easy to reason about: one lock, always the same row, so no deadlock and no ordering
+          argument.
+    - Cons:
+        - It puts a lock, and its timeout handling, on every grade change rather than only on the
+          rare path that lowers a value.
+        - It needs row locks, which SQLite does not support.
+
+    This is the right shape for the paths that lower a value, and Decision 6 uses it there.
+
+4. Use a coarser lock, either one per deployment or one per learner.
+
+    - Pros:
+        - A single lock replaces the ordering argument in Decision 3.
+    - Cons:
+        - A deployment-wide lock serializes every learner behind every other, giving up the
+          throughput bursty grading needs.
+        - The "one per learner" option makes a learner's unrelated competencies wait for each
+          other, since one lock would then cover every tree they have.
+        - Either kind adds machinery for acquiring and releasing locks, and for recovering from a
+          dead lock holder, across a very large key space.
+
+5. Recalculate the derived levels on every read instead of storing them.
+
+    - Pros:
+        - No roll-up writes at all, so there is nothing to keep consistent.
+    - Cons:
+        - It moves a full bottom-up tree evaluation onto every read, the opposite of what dashboards
+          need.
+        - Already settled against in :ref:`openedx-learning-adr-0002`. Unresolved item 1 is the
+          narrower version still open.
+
+6. Send an event to openedx-core and do all the work in a separate celery task.
+
+    - Pros:
+        - Recording a grade would not depend on the competency code being installed or fast.
+    - Cons:
+        - openedx-core is a library and cannot own a celery queue, so every caller would supply one.
+        - The leaf would no longer commit with the grade, giving up the one guarantee Decision 1 is
+          cheap enough to keep.
+
+7. Detect conflicts optimistically, with a version column and a retry loop for the losing write.
+
+    - Pros:
+        - Contention costs a retry rather than a wait.
+    - Cons:
+        - Decision 4 is already optimistic, without the retry loop. A write that loses has computed
+          a value that is too low, and discarding those is exactly what Decision 4 does.
+
+8. Read-after-write: after writing the leaf, re-read each parent's children before rolling up, to catch a race
+   with another writer already in flight.
+
+    - Pros:
+      - Recovers from the race within the same request, without a separate marker or job.
+    - Cons:
+      - Decision 1 shares a transaction only between the grade and its leaf, and Decision 3 commits each rollup level
+        separately before reading the next. That leaves no single transaction boundary for a read-after-write check to
+        run inside: by the time a re-read would happen, the level below has already committed and could change again
+        before the write completes.
+      - It also only checks for a race at the moment each parent is read. If the worker crashes mid-cascade before reaching the next read,
+        nothing notices the rollup was left unfinished. Decision 5's manual recovery mechanism exists to catch that case.