Skip to content

feat: evaluate and record a learner's leaf competency status for a grade - #862

Merged
ormsbee merged 1 commit into
openedx:mainfrom
alezconsultant:alezconsultant/699-record-graded-object-statuses-v2
Oct 16, 2026
Merged

ormsbee merged 1 commit into
openedx:mainfrom
alezconsultant:alezconsultant/699-record-graded-object-statuses-v2

Conversation

@alezconsultant

@alezconsultant alezconsultant commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Description

Implements #699: adds record_graded_object_statuses(*, user_id, scores)
to the CBE applet's public API. Given a learner and one or more graded-object fractions, it
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 mastery status --
raising a stored status but never lowering one, and opening no transaction of its own, so the
caller's grade write and the leaf status commit or roll back together (ADR-0004 Decision 1).

Changes

  • src/openedx_learning/applets/cbe/data.py (new): GradedObjectScore, a frozen, keyword-only
    dataclass carrying object_id: str and fraction: Decimal. Validates its own contract at
    construction: a non-Decimal fraction raises TypeError, an out-of-range, NaN, or infinite
    one raises ValueError.
  • src/openedx_learning/applets/cbe/rule_payloads.py: adds evaluate_rule(rule_type, payload, fraction) -> bool, exported in this module's existing __all__ alongside
    validate_rule_payload. Derives the existing _GRADE_OPERATORS frozenset from the same
    operator-to-function map evaluate_rule uses for comparison, rather than independently, so
    the validator and the evaluator can't drift apart on which operators exist.
  • src/openedx_learning/applets/cbe/api.py: adds the public record_graded_object_statuses,
    plus two private helpers -- _effective_rule (reads whichever of a criterion's assigned
    profile or per-criterion override is set) and _raise_leaf_status (the monotone
    get_or_create + conditional UPDATE ... WHERE status_id < computed write, matching
    ADR-0004 Decision 4's raise-only rule). Both record_graded_object_statuses and
    GradedObjectScore are added to this module's __all__, reaching external callers through
    the existing openedx_learning.api wildcard aggregator with no further changes needed there.
  • tests/openedx_learning/applets/cbe/test_record_graded_object_statuses.py (new): one test per
    acceptance-criteria scenario in the ticket, including both @unit-test-only ones, plus a
    regression test for comparing a Decimal fraction against a rule_payload value Django's
    JSONField returns as a plain float, and coverage for a duplicate object_id, a genuine
    storage error propagating uncaught, and a rolled-back caller transaction leaving no status
    behind.
  • tests/openedx_learning/applets/cbe/test_rule_payloads.py: adds tests for evaluate_rule and
    a regression test guarding the operator-set derivation above.

No new model or migration: this reads only what #641/#642 already define. No .importlinter
changes: every new import stays inside openedx_learning or points down to openedx_tagging.

Manual Testing

Manual testing via django shell/containers. docker exec -it -e PYTHONPATH=/mnt/openedx-core/src tutor_dev-cms-1 ./manage.py cms shell:

from decimal import Decimal
from django.contrib.auth import get_user_model
from openedx_learning.api import GradedObjectScore, record_graded_object_statuses
from openedx_learning.models import (
    CompetencyCriteriaGroup,
    CompetencyCriterion,
    CompetencyTaxonomy,
    MasteryStatus,
    RuleType,
    StudentCompetencyCriterionStatus,
)
from openedx_tagging.models import ObjectTag, Tag

taxonomy = CompetencyTaxonomy.objects.create(name="Test699", export_id="test699")
user, _ = get_user_model().objects.get_or_create(username="test699-learner")

tag_pass = Tag.objects.create(taxonomy=taxonomy, value="Pass")
otag_pass = ObjectTag.objects.create(object_id="block-v1:Test699+C1+2026+problem+pass", taxonomy=taxonomy, tag=tag_pass)
group_pass = CompetencyCriteriaGroup.objects.create(tag=tag_pass)
crit_pass = CompetencyCriterion.objects.create(group=group_pass, object_tag=otag_pass, rule_type_override=RuleType.GRADE, rule_payload_override={"op": "gte", "value": 0.75, "scale": "percent"})

tag_fail = Tag.objects.create(taxonomy=taxonomy, value="Fail")
otag_fail = ObjectTag.objects.create(object_id="block-v1:Test699+C1+2026+problem+fail", taxonomy=taxonomy, tag=tag_fail)
group_fail = CompetencyCriteriaGroup.objects.create(tag=tag_fail)
crit_fail = CompetencyCriterion.objects.create(group=group_fail, object_tag=otag_fail, rule_type_override=RuleType.GRADE, rule_payload_override={"op": "gte", "value": 0.75, "scale": "percent"})

record_graded_object_statuses(user_id=user.id, scores=[GradedObjectScore(object_id=otag_pass.object_id, fraction=Decimal("0.80"))])  # 1
record_graded_object_statuses(user_id=user.id, scores=[GradedObjectScore(object_id=otag_fail.object_id, fraction=Decimal("0.60"))])  # 1

StudentCompetencyCriterionStatus.objects.get(criterion=crit_pass).status_id  # 30 -> MasteryStatus.DEMONSTRATED
StudentCompetencyCriterionStatus.objects.get(criterion=crit_fail).status_id  # 10 -> MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED

Automated:

pytest --no-cov tests/openedx_learning/applets/cbe/test_record_graded_object_statuses.py tests/openedx_learning/applets/cbe/test_rule_payloads.py

Related to #699

🤖 Generated with Claude Code

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 10, 2026
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @alezconsultant!

This repository is currently maintained by @axim-engineering.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@tbain tbain left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this as another PR here and approved it:
jesperhodge#24

@ormsbee ormsbee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tbain already went through this on the fork PR, so I mostly looked at the write path and at how it behaves on MySQL when grade tasks overlap. Claude ran the CBE suite, the quality tools, and makemigrations --check on this branch for me (all clean, and CI is green on MySQL 8).

The code does what #699 asks, the tests cover the acceptance criteria well, and the conditional-update write looks right to me. A few small requests before merge:

  1. It looks like openedx-platform#39179 is going to want to import some models from openedx_learning. Please expose those models in the models_api.py for this app. Otherwise, openedx-platform#39179 will run into import-linter issues.
  2. Please skip archived criteria, now that #857 (afce3e7) is on main (inline on api.py). #699 asked for the skip and for a note on the issue if the field didn't exist yet; the criterion-level field exists now, and the ObjectTag-level one is #856.
  3. Please put the learner and the object in the skipped-criterion warning (inline).

Nits are inline. Anything marked Nit: is optional.

Logistics: #846 has merged, so please rebase onto main. It conflicts on the from .models import line in api.py, and that's the whole conflict: Claude did the trial rebase for me, and with that one line resolved the CBE suite passes (201 tests), makemigrations --check is clean, and the quality checks pass.

Thank you.


Claude drafted this review and ran the test suite, the quality checks, makemigrations --check, and the query probes quoted inline for me on 44ac0e3, and then did the trial rebase onto main once #846 had merged.

Comment thread src/openedx_learning/applets/cbe/api.py Outdated
criteria = (
CompetencyCriterion.objects.filter(
object_tag__object_id__in=list(fraction_by_object_id),
object_tag__tag__taxonomy__competencytaxonomy__isnull=False,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add archived=False to this filter once you rebase onto main (#857 landed as afce3e7 and added the column to CompetencyCriterion), with a test. #699 asked for criteria whose association is archived to be skipped, and for a note on the issue if the field didn't exist yet; the note never got written, and now the criterion-level half of the field does exist. An archived criterion is one an author removed from the tree after some learner already had status for it (ADR-0002 Decision 4, item 7), and I don't think a later grade should create a new status row for it, or raise an existing one, any more than it would for a hard-deleted criterion (@mgwozdz-unicon: does that sound right to you?). #643's rollup is going to have to ignore those rows too, so it's better if they're never written. If the intent is instead that archived criteria keep accumulating status so that un-archiving is lossless, I'd want that written down in ADR-0003, because it's the opposite of what "hidden from new associations" reads as.

@ufedaseyeuconsultant, @mgwozdz-unicon: if #674/#675 archive the leaves whenever they archive a group, the leaf flag is enough here. If a group can be archived with live leaves beneath it, this query needs to look at the group too. I'm not sure what the intended behavior is. Either way, it might make sense to make a custom manager on the model to encapsulate some of that logic in exactly one place as part of the work for those tickets.

Nit, while you're in there: object_tag__taxonomy__competencytaxonomy__isnull=False gives the same answer and skips the join through oel_tagging_tag. Claude printed both queries, and that one INNER JOIN is the only difference, on the no-criteria path that every grade write pays.

Comment thread src/openedx_learning/applets/cbe/api.py
Comment thread src/openedx_learning/applets/cbe/api.py Outdated
Comment thread src/openedx_learning/applets/cbe/api.py
Comment thread src/openedx_learning/applets/cbe/rule_payloads.py
@alezconsultant
alezconsultant force-pushed the alezconsultant/699-record-graded-object-statuses-v2 branch 3 times, most recently from 592c7b0 to fc798a5 Compare October 15, 2026 18:58
@alezconsultant

Copy link
Copy Markdown
Contributor Author

@ormsbee The branch has been rebased, and all conflicts have been resolved. All review nits have been addressed, and the requested fixes have been applied.

@ormsbee ormsbee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick turnaround. Everything from the first round is in, and the answer on #847 (archiving runs every level) means the leaf flag is enough here. Claude re-ran the CBE suite (202 tests), the quality checks, and makemigrations --check on fc798a5 for me, all clean, and CI is green.

Please rebase, add a test for the new atomic-block guard (inline), and this should be good to squash and merge.

Thank you.


Claude drafted this review and re-ran the test suite, the quality checks, and makemigrations --check on fc798a5 for me, plus a probe of the guard outside an atomic block.

: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:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test for this, since it's the one piece of new behavior this round without one. Claude checked that a @pytest.mark.django_db(transaction=True) test sees connection.in_atomic_block as False and gets the TransactionManagementError, and that the same call inside transaction.atomic() goes through, so it's a few lines.

Comment thread src/openedx_learning/models_api.py Outdated
This is also the stable import point for the model class itself, for callers that
need to create competency taxonomies directly.
This is also the stable import point for the model classes themselves, for callers that
need to create CBE rows directly, such as test fixtures.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Comment, no action needed]: Please keep in mind that we want to discourage other apps from creating or mutating these models as much as possible, and even external apps setting up test data should be done through the api.py module. The models_api.py module exists mostly to export models for type annotation.

@alezconsultant
alezconsultant force-pushed the alezconsultant/699-record-graded-object-statuses-v2 branch from fc798a5 to a2aed9b Compare October 16, 2026 18:53
@alezconsultant

Copy link
Copy Markdown
Contributor Author

@ormsbee I've rebased branch, add test, and rephrase comment for models_api to be more explicit about the models_api restrictions.

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#699

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@alezconsultant
alezconsultant force-pushed the alezconsultant/699-record-graded-object-statuses-v2 branch from a2aed9b to 4246da5 Compare October 16, 2026 21:27
@alezconsultant

Copy link
Copy Markdown
Contributor Author

@ormsbee Squashed

@ormsbee
ormsbee merged commit 6005048 into openedx:main Oct 16, 2026
7 checks passed
@github-project-automation github-project-automation Bot moved this from Needs Triage to Done in Contributions Oct 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants