From 4246da56dcb6fdd906d99455856e8ddcdc3c4f82 Mon Sep 17 00:00:00 2001 From: alezconsultant Date: Sat, 17 Oct 2026 01:26:25 +0400 Subject: [PATCH] feat: evaluate and record a learner's leaf competency status for a grade Add record_graded_object_statuses(*, user_id, scores) to the CBE applet's public API: given a learner and one or more graded-object fractions, resolves the competency criteria attached to those objects, evaluates each against its assigned rule profile or per-criterion override, and writes the learner's leaf-level status, raising a stored status but never lowering one. Opens no transaction of its own, so the caller's grade write and the leaf status commit or roll back together, and raises TransactionManagementError when called outside an atomic block. Archived criteria are skipped. The function is marked UNSTABLE. Add GradedObjectScore, a frozen dataclass carrying an object_id and a fraction between 0 and 1, in a new data.py. Add evaluate_rule to the existing rule_payloads.py rather than a new module, deriving its operator set from the same map validate_rule_payload already uses, so the two can never disagree about which operators exist. A criterion whose rule cannot be applied (an unrecognized rule type or an invalid payload) is skipped and logged at warning level with the criterion, learner, and object; the grade write is not affected. Expose the CBE models in openedx_learning.models_api for foreign keys and type annotations. Other apps should create and change these rows only through openedx_learning.api. Related to openedx/openedx-core#699 Co-Authored-By: Claude Sonnet 5 Co-Authored-By: Claude Opus 5.5 --- src/openedx_learning/applets/cbe/api.py | 108 +++- src/openedx_learning/applets/cbe/data.py | 38 ++ .../applets/cbe/rule_payloads.py | 38 +- src/openedx_learning/models_api.py | 17 +- .../cbe/test_record_graded_object_statuses.py | 520 ++++++++++++++++++ .../applets/cbe/test_rule_payloads.py | 59 +- 6 files changed, 771 insertions(+), 9 deletions(-) create mode 100644 src/openedx_learning/applets/cbe/data.py create mode 100644 tests/openedx_learning/applets/cbe/test_record_graded_object_statuses.py diff --git a/src/openedx_learning/applets/cbe/api.py b/src/openedx_learning/applets/cbe/api.py index e2268c297..aa52d33f7 100644 --- a/src/openedx_learning/applets/cbe/api.py +++ b/src/openedx_learning/applets/cbe/api.py @@ -3,9 +3,15 @@ """ from __future__ import annotations +import logging +from datetime import datetime, timezone +from decimal import Decimal +from typing import Any, Sequence + from django.core.exceptions import ValidationError -from django.db import transaction +from django.db import connection, transaction from django.db.models import QuerySet +from django.db.transaction import TransactionManagementError from django.utils.translation import gettext_lazy as _ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import UsageKey @@ -15,25 +21,33 @@ from openedx_tagging.api import create_taxonomy, get_object_tags, tag_object from openedx_tagging.models import ObjectTag, Tag, Taxonomy +from .data import GradedObjectScore from .models import ( CompetencyCriteriaGroup, CompetencyCriterion, CompetencyRuleProfile, CompetencyTaxonomy, LogicOperator, + MasteryStatus, + StudentCompetencyCriterionStatus, ) +from .rule_payloads import evaluate_rule __all__ = [ + "GradedObjectScore", "associate_competency_criterion", "create_competency_criterion", "create_competency_taxonomy", "get_competency_rule_profiles", "is_competency_taxonomy", + "record_graded_object_statuses", "resolve_competency_tag", "resolve_supplied_leaf_group", "select_competency_taxonomies", ] +log = logging.getLogger(__name__) + def create_competency_taxonomy( # pylint: disable=too-many-positional-arguments name: str, @@ -345,3 +359,95 @@ def associate_competency_criterion( rule_type_override=rule_type_override, rule_payload_override=rule_payload_override, ) + + +def _effective_rule(criterion: CompetencyCriterion) -> tuple[str, Any]: + """Return (rule_type, payload) for whichever of criterion's profile or override is set.""" + if criterion.rule_profile_id is not None: + # The oel_cbe_criterion_profile_xor_override_check constraint guarantees rule_profile is + # set whenever rule_profile_id is; this assertion only narrows that fact for mypy. + assert criterion.rule_profile is not None + return criterion.rule_profile.rule_type, criterion.rule_profile.rule_payload + # Same constraint, the other way around: rule_type_override is set whenever rule_profile_id + # is not. + assert criterion.rule_type_override is not None + return criterion.rule_type_override, criterion.rule_payload_override + + +def _raise_leaf_status(user_id: int, criterion_id: int, status: MasteryStatus, now: datetime) -> int: + """ + Create or raise a learner's leaf status row for one criterion; never lowers a stored status. + + Returns the count of rows created or raised (0 or 1), matching the semantics of + ``QuerySet.update()``'s own return value. + """ + row, created = StudentCompetencyCriterionStatus.objects.get_or_create( + user_id=user_id, criterion_id=criterion_id, + defaults={"status_id": status, "created": now, "modified": now}, + ) + if created: + return 1 + return StudentCompetencyCriterionStatus.objects.filter( + pk=row.pk, status_id__lt=status, + ).update(status_id=status, modified=now) + + +def record_graded_object_statuses(*, user_id: int, scores: Sequence[GradedObjectScore]) -> int: + """ + Record a learner's leaf-level competency status for each graded object in ``scores``. + + UNSTABLE: the rollup and learner-status read entry points still to come may change what this + function returns without a deprecation cycle. + + Call this only from inside the same transaction that also writes the grade: this function opens + no transaction of its own and lets any storage error propagate rather than swallowing it, so the + grade and its leaf statuses always commit or roll back together. Pass each ``object_id`` at most + once, using the exact string already stored in ``ObjectTag.object_id``; the match performed + against that value is case-sensitive. Leave out of ``scores`` any object worth zero possible + points and any object the learner has not attempted. Only criteria whose tag belongs to a + competency taxonomy are evaluated, and archived criteria are skipped. An evaluated criterion's + status is only ever raised, never lowered. The return value is the count of leaf rows created or + raised, where ``0`` means nothing changed. This function performs no authorization check of its + own; as with ``openedx_tagging.api``, the caller is responsible for enforcing permissions. + + :raises TransactionManagementError: if called outside an atomic block. + """ + # In autocommit mode get_or_create() would commit a leaf row on its own, apart from the grade. + if not connection.in_atomic_block: + raise TransactionManagementError("record_graded_object_statuses() must run inside the grade's transaction.") + if not scores: + return 0 + + fraction_by_object_id: dict[str, Decimal] = {} + for score in scores: + if score.object_id in fraction_by_object_id: + raise ValueError(f"duplicate object_id in scores: {score.object_id!r}") + fraction_by_object_id[score.object_id] = score.fraction + + criteria = ( + CompetencyCriterion.objects.filter( + object_tag__object_id__in=list(fraction_by_object_id), + object_tag__taxonomy__competencytaxonomy__isnull=False, + archived=False, + ) + .select_related("object_tag", "rule_profile") + .order_by("pk") + ) + + now = datetime.now(tz=timezone.utc) + changed = 0 + for criterion in criteria: + fraction = fraction_by_object_id[criterion.object_tag.object_id] + rule_type, payload = _effective_rule(criterion) + try: + demonstrated = evaluate_rule(rule_type, payload, fraction) + except ValidationError as exc: + log.warning( + "Skipping CompetencyCriterion %s for user %s on object %s: its rule cannot be applied: %s", + criterion.pk, user_id, criterion.object_tag.object_id, exc.messages, + ) + continue + status = MasteryStatus.DEMONSTRATED if demonstrated else MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + changed += _raise_leaf_status(user_id, criterion.pk, status, now) + + return changed diff --git a/src/openedx_learning/applets/cbe/data.py b/src/openedx_learning/applets/cbe/data.py new file mode 100644 index 000000000..a985c6eba --- /dev/null +++ b/src/openedx_learning/applets/cbe/data.py @@ -0,0 +1,38 @@ +""" +Value objects passed across the CBE applet's public API boundary. +""" +from __future__ import annotations + +from dataclasses import dataclass +from decimal import Decimal + + +@dataclass(frozen=True, kw_only=True, slots=True) +class GradedObjectScore: + """ + One learner's final, override-adjusted fraction earned on one graded object. + + ``fraction`` is the caller's final answer for how much of ``object_id`` this learner earned; + this library cannot detect whether an override was already folded into it. Leave an object + worth zero possible points out of the list entirely rather than passing it with a + ``Decimal("0")`` fraction, since it has no meaningful fraction to evaluate. ``Decimal("0")`` + itself is a valid, meaningful input: it means the learner attempted the object and earned + nothing on it. + + ``fraction`` must be built from a string or from ``Decimal`` arithmetic, never from a + ``float``: ``Decimal(0.7)`` is a ``Decimal`` instance, so it passes this class's own type + check, but its value is ``0.69999999999999995559...``, not ``0.7``. + """ + + object_id: str + fraction: Decimal + + def __post_init__(self) -> None: + """Enforce this class's type and range contract once, at construction time.""" + if not isinstance(self.fraction, Decimal): + raise TypeError(f"fraction must be a Decimal, not {type(self.fraction)!r}") + # is_finite() must run first: 0 <= Decimal("NaN") raises decimal.InvalidOperation, not + # ValueError, so checking finiteness first is what makes every invalid input, including + # NaN and Infinity, actually raise the documented ValueError. + if not self.fraction.is_finite() or not 0 <= self.fraction <= 1: + raise ValueError(f"fraction must be finite and between 0 and 1 inclusive, not {self.fraction!r}") diff --git a/src/openedx_learning/applets/cbe/rule_payloads.py b/src/openedx_learning/applets/cbe/rule_payloads.py index 931eba76e..1740152d2 100644 --- a/src/openedx_learning/applets/cbe/rule_payloads.py +++ b/src/openedx_learning/applets/cbe/rule_payloads.py @@ -3,11 +3,14 @@ the shape its rule_type defines. See :ref:`openedx-learning-adr-0002` Decision 3 for the payload contract. ``RuleType`` declares exactly the rule types with a shape defined here, so a rule type can never be offered as a choice without also being saveable. These messages reach an API caller -or admin form, so they must not leak internal class or function names. +or admin form, so they must not leak internal class or function names. ``evaluate_rule()`` applies a +validated payload to a learner's score fraction and returns whether the rule is met. """ from __future__ import annotations -from typing import Literal, TypedDict, get_args +import operator +from decimal import Decimal +from typing import Callable, Literal, TypedDict from django.core.exceptions import ValidationError from django.db import models @@ -16,6 +19,7 @@ __all__ = [ "GradeRulePayload", "RuleType", + "evaluate_rule", "validate_rule_payload", ] @@ -35,7 +39,15 @@ class RuleType(models.TextChoices): GradeOperator = Literal["gte", "lte", "eq"] -_GRADE_OPERATORS: frozenset[str] = frozenset(get_args(GradeOperator)) +# The single source of truth for which operators exist: _GRADE_OPERATORS is derived from this +# map's keys, rather than independently from get_args(GradeOperator), so validate_rule_payload +# and evaluate_rule can never drift apart on which operators are supported. +_GRADE_OPERATOR_FUNCS: dict[str, Callable[[Decimal, Decimal], bool]] = { + "gte": operator.ge, + "lte": operator.le, + "eq": operator.eq, +} +_GRADE_OPERATORS: frozenset[str] = frozenset(_GRADE_OPERATOR_FUNCS) class GradeRulePayload(TypedDict): @@ -107,3 +119,23 @@ def validate_rule_payload(rule_type: str, payload: object) -> None: _("Rule type '%(rule_type)s' is not supported yet; only 'Grade' has a defined rule_payload shape.") % {"rule_type": rule_type} ) + + +def evaluate_rule(rule_type: str, payload: object, fraction: Decimal) -> bool: + """ + Return whether ``fraction`` satisfies ``rule_type``'s ``payload``. + + Validates the payload's shape first (see :func:`validate_rule_payload`); raises + ``ValidationError`` and never returns a value when the shape is invalid, or when + ``rule_type`` has no defined evaluation yet (today, everything except ``RuleType.GRADE``). + """ + validate_rule_payload(rule_type, payload) + if rule_type != RuleType.GRADE: + raise ValidationError( + _("no evaluator is defined yet for rule type '%(rule_type)s'") % {"rule_type": rule_type} + ) + # validate_rule_payload already confirmed payload is a dict with a Grade payload's exact + # keys; this assertion only narrows that fact for mypy, which sees payload as `object`. + assert isinstance(payload, dict) + threshold = Decimal(str(payload["value"])) + return _GRADE_OPERATOR_FUNCS[payload["op"]](fraction, threshold) diff --git a/src/openedx_learning/models_api.py b/src/openedx_learning/models_api.py index 4f02b4f98..998dd99ca 100644 --- a/src/openedx_learning/models_api.py +++ b/src/openedx_learning/models_api.py @@ -1,9 +1,18 @@ """ -Models that we want callers to extend or make foreign keys to. +Models that we want callers to extend, make foreign keys to, or use in type annotations. -This is also the stable import point for the model class itself, for callers that -need to create competency taxonomies directly. +Other apps should create and change these rows, including test data, only through +`openedx_learning.api`, which keeps the invariants that direct model writes would skip. """ # pylint: disable=unused-import -from .models import CompetencyTaxonomy +from .models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyMasteryStatus, + CompetencyRuleProfile, + CompetencyTaxonomy, + MasteryStatus, + RuleType, + StudentCompetencyCriterionStatus, +) diff --git a/tests/openedx_learning/applets/cbe/test_record_graded_object_statuses.py b/tests/openedx_learning/applets/cbe/test_record_graded_object_statuses.py new file mode 100644 index 000000000..c19f5f569 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_record_graded_object_statuses.py @@ -0,0 +1,520 @@ +""" +Tests for `record_graded_object_statuses`, exercising each acceptance-criteria scenario for +leaf-level competency status recording. +""" +import logging +from datetime import datetime, timezone +from decimal import Decimal + +import pytest +from django.db import DatabaseError, transaction +from django.db.transaction import TransactionManagementError +from freezegun import freeze_time + +from openedx_learning.api import GradedObjectScore, record_graded_object_statuses +from openedx_learning.models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyRuleProfile, + CompetencyTaxonomy, + MasteryStatus, + RuleType, + StudentCompetencyCriterionStatus, +) +from openedx_tagging.models import ObjectTag, Tag, Taxonomy + +pytestmark = pytest.mark.django_db + + +def _make_criterion( + *, + group: CompetencyCriteriaGroup, + object_tag: ObjectTag, + rule_profile: CompetencyRuleProfile | None = None, + override: tuple[str, dict] | None = None, +) -> CompetencyCriterion: + """Build a leaf CompetencyCriterion using whichever of rule_profile or override the caller passes.""" + rule_type_override, rule_payload_override = override if override is not None else (None, None) + return CompetencyCriterion.objects.create( + group=group, + object_tag=object_tag, + rule_profile=rule_profile, + rule_type_override=rule_type_override, + rule_payload_override=rule_payload_override, + ) + + +@pytest.fixture(name="criterion") +def _criterion( + group: CompetencyCriteriaGroup, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile +) -> CompetencyCriterion: + """A leaf CompetencyCriterion directly under the root `group`, using the seeded default profile.""" + return _make_criterion(group=group, object_tag=object_tag, rule_profile=default_rule_profile) + + +@pytest.fixture(name="rule_profile_75") +def _rule_profile_75(competency_taxonomy: CompetencyTaxonomy) -> CompetencyRuleProfile: + """A CompetencyRuleProfile scoped to `competency_taxonomy`, requiring at least 0.75.""" + return CompetencyRuleProfile.objects.create( + competency_taxonomy=competency_taxonomy, + rule_type=RuleType.GRADE, + rule_payload={"op": "gte", "value": 0.75, "scale": "percent"}, + ) + + +def test_grade_meeting_threshold_demonstrates( + user, group: CompetencyCriteriaGroup, object_tag: ObjectTag, rule_profile_75: CompetencyRuleProfile +) -> None: + """A grade of 80% against a 75% threshold demonstrates the criterion.""" + criterion = _make_criterion(group=group, object_tag=object_tag, rule_profile=rule_profile_75) + + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.80"))], + ) + + assert changed == 1 + assert StudentCompetencyCriterionStatus.objects.get(criterion=criterion).status_id == MasteryStatus.DEMONSTRATED + + +def test_grade_below_threshold_is_attempted_not_demonstrated( + user, group: CompetencyCriteriaGroup, object_tag: ObjectTag, rule_profile_75: CompetencyRuleProfile +) -> None: + """A grade of 60% against a 75% threshold is attempted but not demonstrated.""" + criterion = _make_criterion(group=group, object_tag=object_tag, rule_profile=rule_profile_75) + + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.60"))], + ) + + assert changed == 1 + assert ( + StudentCompetencyCriterionStatus.objects.get(criterion=criterion).status_id + == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + ) + + +@pytest.mark.parametrize( + "fraction, expected_status", + [ + pytest.param(Decimal("0.74"), MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED, id="just_below"), + pytest.param(Decimal("0.76"), MasteryStatus.DEMONSTRATED, id="just_above"), + pytest.param(Decimal("0.749999999999"), MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED, id="fractionally_below"), + pytest.param(Decimal("0.7500000000"), MasteryStatus.DEMONSTRATED, id="exact_with_trailing_zeros"), + pytest.param(Decimal("0.7500000001"), MasteryStatus.DEMONSTRATED, id="fractionally_above"), + ], +) +def test_threshold_boundary_precision( + user, group: CompetencyCriteriaGroup, object_tag: ObjectTag, rule_profile_75: CompetencyRuleProfile, + *, fraction: Decimal, expected_status: MasteryStatus, +) -> None: + """ + Decimal arithmetic is exact, so a fraction on either side of a 0.75 threshold -- including at + extreme precision, and including one with trailing zeros -- resolves without rounding error. + """ + criterion = _make_criterion(group=group, object_tag=object_tag, rule_profile=rule_profile_75) + + record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=fraction)], + ) + + assert StudentCompetencyCriterionStatus.objects.get(criterion=criterion).status_id == expected_status + + +def test_score_exactly_at_default_threshold_demonstrates( + user, criterion: CompetencyCriterion, object_tag: ObjectTag +) -> None: + """ + A score of exactly Decimal("0.8") against the seeded default profile (at least 0.8) demonstrates + the criterion. This is the regression test for comparing a Decimal fraction against a rule_payload + value that Django's JSONField returns as a plain float. + """ + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.8"))], + ) + + assert changed == 1 + assert StudentCompetencyCriterionStatus.objects.get(criterion=criterion).status_id == MasteryStatus.DEMONSTRATED + + +def test_object_with_no_criteria_costs_one_query_and_writes_nothing(user, django_assert_num_queries) -> None: + """Content with no competency criteria is unaffected, costing exactly one query.""" + score = GradedObjectScore(object_id="block-v1:Org1+Python100+Fall2026+problem+none", fraction=Decimal("0.9")) + with django_assert_num_queries(1): + changed = record_graded_object_statuses(user_id=user.id, scores=[score]) + + assert changed == 0 + assert not StudentCompetencyCriterionStatus.objects.exists() + + +def test_object_tagged_with_unrelated_tag_writes_nothing(user) -> None: + """A subsection tagged with a tag no criterion points at is unaffected.""" + plain_taxonomy = Taxonomy.objects.create(name="Plain Tags", export_id="plain-v1") + plain_tag = Tag.objects.create(taxonomy=plain_taxonomy, value="Unrelated") + plain_object_tag = ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+unrelated", taxonomy=plain_taxonomy, tag=plain_tag, + ) + + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=plain_object_tag.object_id, fraction=Decimal("0.9"))], + ) + + assert changed == 0 + assert not StudentCompetencyCriterionStatus.objects.exists() + + +def test_empty_scores_costs_no_queries(user, django_assert_num_queries) -> None: + """An empty scores sequence returns 0 and issues no queries at all.""" + with django_assert_num_queries(0): + changed = record_graded_object_statuses(user_id=user.id, scores=[]) + + assert changed == 0 + + +def test_one_score_evaluates_each_criterion_on_its_own_threshold( + user, competency_taxonomy: CompetencyTaxonomy, group: CompetencyCriteriaGroup, object_tag: ObjectTag, + rule_profile_75: CompetencyRuleProfile, +) -> None: + """ + One grade satisfies two criteria with different thresholds: two competency tags each carry their + own ObjectTag sharing one object_id, one criterion resolved via a profile, the other via a + per-criterion override, and a single score demonstrates one but not the other. + """ + other_tag = Tag.objects.create(taxonomy=competency_taxonomy, value="Other Competency") + other_object_tag = ObjectTag.objects.create( + object_id=object_tag.object_id, taxonomy=competency_taxonomy, tag=other_tag, + ) + other_group = CompetencyCriteriaGroup.objects.create(tag=other_tag) + criterion = _make_criterion(group=group, object_tag=object_tag, rule_profile=rule_profile_75) + other_criterion = _make_criterion( + group=other_group, object_tag=other_object_tag, + override=(RuleType.GRADE, {"op": "gte", "value": 0.9, "scale": "percent"}), + ) + + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.8"))], + ) + + assert changed == 2 + assert StudentCompetencyCriterionStatus.objects.get(criterion=criterion).status_id == MasteryStatus.DEMONSTRATED + assert ( + StudentCompetencyCriterionStatus.objects.get(criterion=other_criterion).status_id + == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + ) + + +def test_lower_grade_does_not_lower_demonstrated( + user, criterion: CompetencyCriterion, object_tag: ObjectTag +) -> None: + """A later, lower grade does not take away an already-demonstrated status.""" + first = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.8"))], + ) + assert first == 1 + + second = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.6"))], + ) + + assert second == 0 + assert StudentCompetencyCriterionStatus.objects.get(criterion=criterion).status_id == MasteryStatus.DEMONSTRATED + + +def test_higher_grade_raises_attempted_to_demonstrated( + user, criterion: CompetencyCriterion, object_tag: ObjectTag +) -> None: + """A later, higher grade raises a stored status, and only `modified` moves.""" + first_time = datetime(2026, 1, 1, tzinfo=timezone.utc) + second_time = datetime(2026, 1, 2, tzinfo=timezone.utc) + + with freeze_time(first_time): + first = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.6"))], + ) + assert first == 1 + + with freeze_time(second_time): + second = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.8"))], + ) + assert second == 1 + + row = StudentCompetencyCriterionStatus.objects.get(criterion=criterion) + assert row.status_id == MasteryStatus.DEMONSTRATED + assert row.created == first_time + assert row.modified == second_time + + +def test_identical_repeat_changes_nothing(user, criterion: CompetencyCriterion, object_tag: ObjectTag) -> None: + """Repeating an identical evaluation changes nothing, including `modified`.""" + first = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.9"))], + ) + assert first == 1 + modified_after_first = StudentCompetencyCriterionStatus.objects.get(criterion=criterion).modified + + second = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.9"))], + ) + + assert second == 0 + assert StudentCompetencyCriterionStatus.objects.get(criterion=criterion).modified == modified_after_first + + +@pytest.mark.parametrize( + "corrupt_kwargs", + [ + pytest.param({"rule_type_override": "MasteryLevel"}, id="unsupported_rule_type"), + pytest.param( + {"rule_payload_override": {"op": "invalid_op", "value": 0.8, "scale": "percent"}}, + id="invalid_operator", + ), + ], +) +def test_unappliable_rule_is_skipped_and_logged( + user, criterion: CompetencyCriterion, object_tag: ObjectTag, + caplog: pytest.LogCaptureFixture, corrupt_kwargs: dict, +) -> None: + """ + A criterion whose rule cannot be applied is passed over: the well-formed sibling criterion still + gets its row and the function still returns 1, and a WARNING names the skipped criterion, the + learner, and the object. + + The bad criterion is first created with a valid override, then corrupted with a direct queryset + `update()`, which bypasses `CompetencyCriterion.save()`'s own validation: `save()` itself would + reject either corrupted shape. + """ + competency_taxonomy = object_tag.taxonomy + bad_tag = Tag.objects.create(taxonomy=competency_taxonomy, value="Bad Rule") + bad_group = CompetencyCriteriaGroup.objects.create(tag=bad_tag) + bad_object_tag = ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+bad", taxonomy=competency_taxonomy, tag=bad_tag, + ) + bad_criterion = _make_criterion( + group=bad_group, object_tag=bad_object_tag, + override=(RuleType.GRADE, {"op": "gte", "value": 0.8, "scale": "percent"}), + ) + CompetencyCriterion.objects.filter(pk=bad_criterion.pk).update(**corrupt_kwargs) + + with caplog.at_level(logging.WARNING, logger="openedx_learning.applets.cbe.api"): + changed = record_graded_object_statuses( + user_id=user.id, + scores=[ + GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.9")), + GradedObjectScore(object_id=bad_object_tag.object_id, fraction=Decimal("0.9")), + ], + ) + + assert changed == 1 + assert StudentCompetencyCriterionStatus.objects.get(criterion=criterion).status_id == MasteryStatus.DEMONSTRATED + assert not StudentCompetencyCriterionStatus.objects.filter(criterion=bad_criterion).exists() + assert any( + record.levelname == "WARNING" + and f"CompetencyCriterion {bad_criterion.pk} for user {user.id} on object {bad_object_tag.object_id}" + in record.getMessage() + for record in caplog.records + ) + + +def test_invalid_profile_payload_is_skipped( + user, criterion: CompetencyCriterion, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile, + caplog: pytest.LogCaptureFixture, +) -> None: + """ + A criterion resolved through a profile whose stored payload is invalid is skipped and logged, the + same as an invalid per-criterion override. The profile is corrupted with a direct queryset `update()`, + since `save()` itself would reject an invalid payload. + """ + CompetencyRuleProfile.objects.filter(pk=default_rule_profile.pk).update( + rule_payload={"op": "invalid_op", "value": 0.8, "scale": "percent"}, + ) + + with caplog.at_level(logging.WARNING, logger="openedx_learning.applets.cbe.api"): + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.9"))], + ) + + assert changed == 0 + assert not StudentCompetencyCriterionStatus.objects.filter(criterion=criterion).exists() + assert any( + record.levelname == "WARNING" and str(criterion.pk) in record.getMessage() + for record in caplog.records + ) + + +def test_criterion_on_non_competency_taxonomy_tag_is_not_evaluated( + user, group: CompetencyCriteriaGroup, default_rule_profile: CompetencyRuleProfile +) -> None: + """ + A criterion on a tag outside a competency taxonomy is not evaluated, even when its parent group + sits on a competency-taxonomy tag. + """ + plain_taxonomy = Taxonomy.objects.create(name="Plain Tags", export_id="plain-v2") + plain_tag = Tag.objects.create(taxonomy=plain_taxonomy, value="Plain") + plain_object_tag = ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+plain", taxonomy=plain_taxonomy, tag=plain_tag, + ) + _make_criterion(group=group, object_tag=plain_object_tag, rule_profile=default_rule_profile) + + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=plain_object_tag.object_id, fraction=Decimal("0.9"))], + ) + + assert changed == 0 + assert not StudentCompetencyCriterionStatus.objects.exists() + + +def test_omitted_zero_point_object_gets_no_status( + user, criterion: CompetencyCriterion, object_tag: ObjectTag, competency_taxonomy: CompetencyTaxonomy, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """A subsection worth zero points is simply omitted from scores, and only the scored object gets a status.""" + other_tag = Tag.objects.create(taxonomy=competency_taxonomy, value="Unscored") + other_object_tag = ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+unscored", taxonomy=competency_taxonomy, tag=other_tag, + ) + other_criterion = _make_criterion( + group=CompetencyCriteriaGroup.objects.create(tag=other_tag), + object_tag=other_object_tag, + rule_profile=default_rule_profile, + ) + + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.9"))], + ) + + assert changed == 1 + assert StudentCompetencyCriterionStatus.objects.filter(criterion=criterion).exists() + assert not StudentCompetencyCriterionStatus.objects.filter(criterion=other_criterion).exists() + + +def test_rolled_back_grade_leaves_no_status( + user, criterion: CompetencyCriterion, object_tag: ObjectTag +) -> None: + """A grade that is not saved leaves no competency status behind, since this function opens no transaction.""" + + class _BoomError(Exception): + """Raised deliberately to force the enclosing transaction to roll back.""" + + with pytest.raises(_BoomError), transaction.atomic(): + record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.9"))], + ) + raise _BoomError() + + assert not StudentCompetencyCriterionStatus.objects.filter(criterion=criterion).exists() + + +@pytest.mark.django_db(transaction=True) +def test_call_outside_an_atomic_block_raises() -> None: + """Called in autocommit mode it raises, and the same call inside an atomic block goes through.""" + scores = [GradedObjectScore(object_id="block-v1:Org+Course+Run+type@problem+block@p1", fraction=Decimal("0.9"))] + + with pytest.raises(TransactionManagementError): + record_graded_object_statuses(user_id=1, scores=scores) + + with transaction.atomic(): + assert record_graded_object_statuses(user_id=1, scores=scores) == 0 + + +def test_storage_error_propagates( + user, criterion: CompetencyCriterion, object_tag: ObjectTag, monkeypatch: pytest.MonkeyPatch, +) -> None: + """A genuine storage failure propagates out uncaught, rather than being mistaken for an unappliable rule.""" + def _boom(*args, **kwargs): + raise DatabaseError("boom") + + monkeypatch.setattr(StudentCompetencyCriterionStatus.objects, "get_or_create", _boom) + + with pytest.raises(DatabaseError): + record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.9"))], + ) + + assert not StudentCompetencyCriterionStatus.objects.filter(criterion=criterion).exists() + + +def test_duplicate_object_id_raises_value_error(user, django_assert_num_queries) -> None: + """A repeated object_id in one call raises ValueError before any query runs.""" + scores = [ + GradedObjectScore(object_id="dup", fraction=Decimal("0.5")), + GradedObjectScore(object_id="dup", fraction=Decimal("0.9")), + ] + + with django_assert_num_queries(0): + with pytest.raises(ValueError): + record_graded_object_statuses(user_id=user.id, scores=scores) + + +def test_graded_object_score_validation() -> None: + """ + Decimal("0") and Decimal("1") are valid; a fraction outside 0-1, NaN, or Infinity raises ValueError; + a plain float raises TypeError. + """ + GradedObjectScore(object_id="x", fraction=Decimal("0")) + GradedObjectScore(object_id="x", fraction=Decimal("1")) + + for bad_fraction in (Decimal("-0.01"), Decimal("1.01"), Decimal("NaN"), Decimal("Infinity")): + with pytest.raises(ValueError): + GradedObjectScore(object_id="x", fraction=bad_fraction) + + with pytest.raises(TypeError): + GradedObjectScore(object_id="x", fraction=0.8) # type: ignore[arg-type] + + +def test_archived_rule_profile_still_evaluates( + user, competency_taxonomy: CompetencyTaxonomy, group: CompetencyCriteriaGroup, object_tag: ObjectTag +) -> None: + """ + An archived CompetencyRuleProfile remains queryable and is still evaluated normally, per ADR-0002 + Decision 3. + """ + profile = CompetencyRuleProfile.objects.create( + competency_taxonomy=competency_taxonomy, rule_type=RuleType.GRADE, + rule_payload={"op": "gte", "value": 0.75, "scale": "percent"}, + ) + criterion = _make_criterion(group=group, object_tag=object_tag, rule_profile=profile) + profile.archived = True + profile.save() + + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.8"))], + ) + + assert changed == 1 + assert StudentCompetencyCriterionStatus.objects.get(criterion=criterion).status_id == MasteryStatus.DEMONSTRATED + + +def test_archived_criterion_is_skipped( + user, group: CompetencyCriteriaGroup, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile +) -> None: + """ + An archived criterion neither gets a new status row nor has an existing one raised, per ADR-0002 + Decision 4: archiving removes it from the tree. + """ + tag = object_tag.tag + other_object_tag = ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+other", taxonomy=object_tag.taxonomy, tag=tag, + ) + with_status = _make_criterion(group=group, object_tag=object_tag, rule_profile=default_rule_profile) + without_status = _make_criterion(group=group, object_tag=other_object_tag, rule_profile=default_rule_profile) + record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.1"))], + ) + for criterion in (with_status, without_status): + criterion.archived = True + criterion.save() + + changed = record_graded_object_statuses( + user_id=user.id, + scores=[ + GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("1")), + GradedObjectScore(object_id=other_object_tag.object_id, fraction=Decimal("1")), + ], + ) + + assert changed == 0 + assert ( + StudentCompetencyCriterionStatus.objects.get(criterion=with_status).status_id + == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + ) + assert not StudentCompetencyCriterionStatus.objects.filter(criterion=without_status).exists() diff --git a/tests/openedx_learning/applets/cbe/test_rule_payloads.py b/tests/openedx_learning/applets/cbe/test_rule_payloads.py index 47c342793..555726c27 100644 --- a/tests/openedx_learning/applets/cbe/test_rule_payloads.py +++ b/tests/openedx_learning/applets/cbe/test_rule_payloads.py @@ -5,10 +5,19 @@ "Grade", whose payload is {"op": ..., "value": ..., "scale": ...} where op is one of gte, lte or eq, value is a fraction from 0.0 to 1.0 rather than a number out of 100, and scale is "percent". """ +from decimal import Decimal + import pytest from django.core.exceptions import ValidationError -from openedx_learning.applets.cbe.rule_payloads import GradeRulePayload, RuleType, validate_rule_payload +from openedx_learning.applets.cbe.rule_payloads import ( + _GRADE_OPERATOR_FUNCS, + _GRADE_OPERATORS, + GradeRulePayload, + RuleType, + evaluate_rule, + validate_rule_payload, +) _GRADE_PAYLOAD: GradeRulePayload = {"op": "gte", "value": 0.8, "scale": "percent"} @@ -128,3 +137,51 @@ def test_every_rule_type_choice_has_a_validation_branch(rule_type: RuleType) -> validate_rule_payload(rule_type, {}) assert "not supported yet" not in " ".join(exc_info.value.messages) + + +@pytest.mark.parametrize( + "op, fraction, expected", + [ + pytest.param("gte", Decimal("0.81"), True, id="gte_above"), + pytest.param("gte", Decimal("0.8"), True, id="gte_exact_threshold"), + pytest.param("gte", Decimal("0.79"), False, id="gte_below"), + pytest.param("lte", Decimal("0.8"), True, id="lte_exact_threshold"), + pytest.param("lte", Decimal("0.79"), True, id="lte_below"), + pytest.param("lte", Decimal("0.81"), False, id="lte_above"), + pytest.param("eq", Decimal("0.8"), True, id="eq_exact"), + pytest.param("eq", Decimal("0.79"), False, id="eq_not_exact"), + ], +) +def test_evaluate_rule(op: str, fraction: Decimal, expected: bool) -> None: + """ + evaluate_rule() applies each supported operator correctly, including the exact-threshold case + that regresses comparing a Decimal fraction against the float this rule_payload's "value" comes + back as from a JSONField: gte 0.8 against exactly Decimal("0.8") must return True, not False. + """ + payload = {**_GRADE_PAYLOAD, "op": op} + assert evaluate_rule(RuleType.GRADE, payload, fraction) is expected + + +@pytest.mark.parametrize( + "rule_type, payload", + [ + pytest.param("MasteryLevel", {"level": 3}, id="unsupported_rule_type"), + pytest.param(RuleType.GRADE, {"op": "startswith", "value": 0.8, "scale": "percent"}, id="unsupported_operator"), + ], +) +def test_evaluate_rule_raises_for_unappliable_rule(rule_type: str, payload: dict) -> None: + """ + ValidationError propagates out of evaluate_rule() uncaught for an unsupported rule type or + operator. validate_rule_payload() itself already has thorough, separate coverage above; this + only confirms evaluate_rule() surfaces the same exception, not that the validator is correct. + """ + with pytest.raises(ValidationError): + evaluate_rule(rule_type, payload, Decimal("0.8")) + + +def test_grade_operators_frozenset_matches_evaluator_map() -> None: + """ + _GRADE_OPERATORS is derived from _GRADE_OPERATOR_FUNCS's own keys, so the two cannot drift apart: + a future operator added to one without the other would fail this test immediately. + """ + assert _GRADE_OPERATORS == frozenset(_GRADE_OPERATOR_FUNCS)