Skip to content

docs: cbe competency rule profile adr amendments#636

Open
mgwozdz-unicon wants to merge 3 commits into
openedx:mainfrom
mgwozdz-unicon:cbe-competency-rule-profile-adr-amendments
Open

docs: cbe competency rule profile adr amendments#636
mgwozdz-unicon wants to merge 3 commits into
openedx:mainfrom
mgwozdz-unicon:cbe-competency-rule-profile-adr-amendments

Conversation

@mgwozdz-unicon

Copy link
Copy Markdown
Contributor

Claude assisted...

Summary

Amends ADR 0002 (competency criteria model) and ADR 0003 (versioning) to close gaps in the CompetencyRuleProfile design in this Slack thread: https://openedx.slack.com/archives/C09A1BN1CSE/p1783010474048009

Changes

ADR 0002

  • CompetencyRuleProfile scope fields (organization_id, course_id, competency_taxonomy_id) are now consistently nullable, at most one populated per row; combining more than one on a single profile is explicitly out of scope for now (documented as a new Rejected Alternative, since a required-parent-scope hierarchy was considered and doesn't fit taxonomies that span multiple organizations).
  • Added a unique constraint on the scope-column index, so at most one profile can exist per distinct scope combination.
  • Added CompetencyTaxonomy.taxonomy_overrides_org, a boolean (default false) that breaks the tie when both an organization-scoped and a taxonomy-scoped profile could apply to the same criterion.
  • Rewrote how CompetencyCriterion.competency_rule_profile_id gets assigned: it's recomputed at several explicit trigger points (creation, a more specific profile appearing later, a per-criterion override being set, an override being cleared back to the default) rather than resolved dynamically at evaluation time. Added the resolution-order tables (by which profile rows exist) and the org/taxonomy tiebreak table directly to the ADR.
  • CompetencyRuleProfile is now never hard-deleted, even pre-use (a carve-out from the general delete-protection rule) — retirement is archive-only via a new archived column. Editing is restricted to rule_type/rule_payload; scope fields are immutable after creation.
  • Corrected the Grade rule payload example to the platform's actual fractional grade scale (0.0–1.0, not 0–100), and specified the allowed op values (gte, lte, eq).

ADR 0003

  • Decision 4 now defines "in use" for a CompetencyRuleProfile explicitly, and extends the existing edit-warning requirement to cover profile reassignment and override-state switching, not just direct edits.

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

Copy link
Copy Markdown

Thanks for the pull request, @mgwozdz-unicon!

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.

@github-project-automation github-project-automation Bot moved this to Needs Triage in Contributions Jul 3, 2026
@mphilbrick211 mphilbrick211 moved this from Needs Triage to Ready for Review in Contributions Jul 6, 2026
@mphilbrick211 mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Jul 6, 2026

@jesperhodge jesperhodge 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 left some questions.
On a high level, I wonder instead of rule profiles if it's not better to allow course authors to define rules and assign them to whatever they want, including but not limited to the whole course - see my comments for more detail.

Relationship to other concepts:

- Can be scoped by taxonomy, course, and/or organization.
- Each row is scoped by at most one of taxonomy, course, or organization (or by none, for the system default). See Decision 4 for how a criterion is assigned a profile when rows in more than one of these scopes could apply to it.

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.

Question: if taxonomy + course + organization are all none, this is system default. Does uniqueness apply to the system default as well, i.e. there can only be one system default rule profile?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, there should only be 1 system default rule profile.


This new database table will have the following columns:
1. ``taxonomy_ptr_id``: Primary key and one-to-one foreign key to ``oel_tagging_taxonomy.id``.
2. ``taxonomy_overrides_org``: Boolean, defaults to ``false``. Used only while computing which single ``CompetencyRuleProfile`` to assign to a ``CompetencyCriterion`` (Decision 4). If, for a criterion's context, both an organization-scoped profile row and a taxonomy-scoped profile row exist as candidates, this field decides which one gets assigned: ``false`` (default) assigns the organization-scoped row; ``true`` assigns this taxonomy's own row instead, so it cannot be overridden by an organization. Once assigned, the criterion stores that one profile's id and this field plays no further part. Not exercised in this phase; see the MVP note in Decision 4.

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.

Questions:

  • the taxonomy here decides if its own competency rule profile should be overridden by the org course profile if present or not. Why is this decided by the taxonomy?
  • "Not exercised in this phase" - what does that mean for MVP implementation? Create but not use this field?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm drafting some clarifications to this in the ADR now, but to answer your questions:

  1. Why the taxonomy decides the override: a single taxonomy can compete against many organization-scoped profiles, so the tiebreak needs one consistent answer that applies to all of them. Putting it on the taxonomy avoids setting (and keeping in sync) the same flag on every org-scoped profile row. Added as a new bullet under Decision 1's "Relationship to other concepts."

  2. "Not exercised in this phase" means: the column is created and defaults to false, but no code reads it yet, since organization-scoped profiles aren't created in this phase (see the MVP note in Decision 4), so the conflict it resolves can't occur yet. Reworded that sentence in the column description to say this directly.

Relationship to other concepts:

- Can be scoped by taxonomy, course, and/or organization.
- Each row is scoped by at most one of taxonomy, course, or organization (or by none, for the system default). See Decision 4 for how a criterion is assigned a profile when rows in more than one of these scopes could apply to it.

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.

Question:
Trying to understand how this works in practice. Let's say for example we have a course, and this course has various assignments. Some of these assignments demonstrate the "Python" skill, some of these demonstrate the "JS" skill. So we make sure the correct competency taxonomy that contains different skills is linked (is this linking done by the course author?)
Then the author can use AND/OR statements to apply the competency rule profile to different assignments.
Since there is a course-wide or taxonomy-wide rule profile, initially the taxonomy decides that all assignments need to have gte 75% of success (why is this specified for the taxonomy, as it applies to assignments in a course?) or the author can set a taxonomy rule profile for the course stating that all assignments need to meet 75% of success, and he can override this per assignment if desired.
But unless the author manually edits assignment rules, they can't set e.g. 75% for Python assignments, 50% for JS assignments, and 65% for take-home assignments in Python?
So... how is having one rule-profile per course or even per taxonomy helpful? Why don't they differentiate by skill or assignment type or something else?

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.

Alternatively: why have this complicated construct if it can create hurdles (like not distinguishing between skill and assignment types)?
Why not like this?

  • rule_type and rule_payload can be set per assignment if desired, like before, overriding any course-wide or org-wide values or values that are set up higher in the criteria tree (similar to how problem grading works now)
  • The author can define a CompetencyRule that he assigns to one or more assignments/tag applications (criterion), or to a skill tag via CompetencyCriteriaGroup / to a tree node, or to the whole course or multiple courses. That relationship can be reflected via another table
  • Even CompetencyRules that are applied to a whole course can be marked to apply only to specific skills or assignment types
  • No more fixed inflexible rule profiles - the author just creates a rule and assigns it to whatever he wants, and being able to apply it to the whole course or even org allows him to be efficient.

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.

My point is mute (see #636 (review)).

@jesperhodge jesperhodge 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.

The ADR looks good to me, from my side it's ready.

The points from my previous review have been answered, and they are not relevant:

CBE Courses are already designed in a way where it is intended for you to have one grade threshold across the board for a course, or sometimes different rules for different taxonomies.
But they generally don't differentiate per skill, per assignment type, and such.

Nit: as discussed, adding some additional explanation for this will be helpful, just for background. But don't let that block this PR.

7. ``StudentCompetencyCriteriaGroupStatus(user_id, competency_criteria_group_id)``
8. ``StudentCompetencyStatus(user_id, oel_tagging_tag_id)``
9. ``CompetencyRuleProfile(competency_taxonomy_id, course_id, organization_id)``
9. ``CompetencyRuleProfile(competency_taxonomy_id, course_id, organization_id)`` (unique -- at most one profile per distinct scope combination)

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.

How are we ensuring unique combinations if there are nullable fields?

- Explain why taxonomy_overrides_org lives on the taxonomy rather
  than the org-scoped profile (Decision 1).
- Clarify that the field is created but unread this phase, and why.
- Add a generated scope_key column and check constraint so
  CompetencyRuleProfile scope uniqueness is actually enforced on
  every supported backend; a plain unique constraint on the three
  nullable scope columns silently does nothing on MySQL, and a
  conditional/partial constraint isn't supported there either
  (Decision 3, Decision 5, Rejected Alternatives).
- Allow the system default profile's rule_type/rule_payload to be
  edited through the profile API; it remains migration-seeded and
  cannot be created or deleted through the API.
- Clarify in Context that the system-wide default is the common
  case, with taxonomy/organization/course/criterion scoping and
  overrides handling progressively narrower exceptions.
@mgwozdz-unicon

Copy link
Copy Markdown
Contributor Author

My latest commit addresses the open review threads, plus one design gap two of them turned out to share.

Jesper, taxonomy_overrides_org field (Decision 1): added a bullet explaining why the flag lives on the taxonomy rather than the org-scoped profile: a single taxonomy can compete against many org-scoped profiles, so the tiebreak needs one answer that applies to all of them, not one duplicated per org.

Jesper, "Not exercised in this phase" (Decision 1): reworded that sentence. The field is created and defaults to false, but nothing reads it yet since org-scoped profiles aren't created in this phase, so the conflict it resolves can't occur.

Jesper, system default uniqueness (Decision 3): confirmed only one system default row should ever exist, but this surfaced a real gap: a plain unique constraint across nullable scope columns doesn't enforce that in SQL, NULL is never equal to NULL, and our MySQL backend doesn't support the usual partial/conditional constraint workaround either. Fixed by adding a generated scope_key column that collapses the three scope columns into one comparable value, with an ordinary unique constraint on it (Decision 3, Decision 5). Also added a check constraint so at most one of the three raw scope columns can be set per row.

Dave, same gap on the index list (Decision 5): same fix as above. Also documented the rejected partial-constraint approach in Rejected Alternatives so it doesn't get re-proposed later.

Jesper, background on why this feature exists (Context): per our conversation, the Context section now states plainly that in the common case, the mastery rule is the same across the whole instance (the system-wide default), and that taxonomy, organization, course, and criterion scoping/overrides exist to handle progressively narrower exceptions to that default, not the typical case.

Also: made the system default profile editable (rule_type/rule_payload) through the profile API. It's still only ever created or deleted via the migration seed, not through the API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

FC Relates to an Axim Funded Contribution project open-source-contribution PR author is not from Axim or 2U

Projects

Status: Ready for Review

Development

Successfully merging this pull request may close these issues.

5 participants