docs: ADR 0004 competency mastery concurrency - #713
Conversation
Adds ADR 0004 covering how learner competency mastery is recorded under concurrent, out-of-order grade-change events without a per-event serialization cost. Adjusts ADRs 0002 and 0003 to match: learner status is stored as an in-place ACTIVE row plus a paired append-only HISTORY table, with a unique index on the leaf HISTORY advance that serves as the idempotency key for the append. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @jesperhodge! This repository is currently maintained by 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 approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo 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:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere 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:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
|
Two things stood out reading through this. Monotonicity override. ADR 0003's reworded Decision 5 says downward status adjustments "are prohibited," as a flat rule with no exception. ADR 0005 (not part of this PR) has "Reversing a banked status is a separate administrative action, out of scope here," which reads as: overrides are permitted, just not through the normal recorder path and not spec'd here. Since 0005 isn't in this PR, ADR 0003 currently reads as an absolute prohibition with no hint that an override path exists or is intentionally deferred elsewhere. Worth adding a line noting that admin-initiated corrections are out of scope for this ADR rather than prohibited outright, even before 0005's mechanism is settled. ADR 0005 coupling. I went through every A few ways to handle it:
I'd lean toward the middle option: pull the 0002/0003 HISTORY-table content back into 0005 where it actually belongs, and reword ADR 0004's three load-bearing spots as assumptions this ADR needs rather than facts cited from an unmerged decision. That keeps this PR mergeable on its own terms and limits the blast radius if the history-storage direction changes. |
Whether learner status history is stored as separate append-only tables, and whether those are advance-only, is still under discussion. Remove the decision from ADRs 0002, 0003, and 0004 rather than commit to it. ADR 0004 now states only that the competency mastery status writes and the roll-ups commit in the same transaction as the subsection grade, without specifying what else that transaction may carry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Monotonicity is a property of the automatic path, not an invariant of the stored data. Grade changes and competency criteria rule changes never lower a status, but staff can, through Django admin or as a deliberate instructor correction, and a direct edit cascades to the ancestors above the edited node. Add an Open Questions section recording that the cascade decision needs confirmation, and that whether a cascade may overwrite a hand-set ancestor status is still undecided. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The row lock taken by the merge UPDATE is held until the transaction commits, not for the duration of the statement. Mechanism 2's lock ordering argument depends on that, so state it correctly. Give the deadlock-freedom argument its missing premise, that the criteria tree gives every node exactly one parent, and close the gap where one grade change advances several leaves at once by fixing their lock order. Attribute the READ COMMITTED default to Django's MySQL backend, which is what actually sets it, and drop the unenforced claim that higher isolation levels are unsupported. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Break the dense Context and Decision paragraphs into one idea each, cut the nested parentheticals, and drop the write-skew jargon in favor of the plain description already alongside it. Replace mechanism 3's vague "generalized as needed to other places" with what it means: further entry points will call the same function. Content is unchanged. Every mechanism, the monotonicity carve-out for direct staff edits, and the isolation-level argument all say what they said before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
On the two open questions: I'd say yes to both. A direct staff edit should cascade to ancestors, and a cascade may overwrite a hand-set ancestor. Decision 4. This one lost its own idempotency argument, the unique constraint on learner/node/status and its cross-reference to ADR 0002, independent of removing item 5. Was that intentional, or a side effect of editing around item 5's removal? Decision 2. The first paragraph ends on an unresolved problem, "each recomputation could read the other child's old value and write a parent status that is too low," and the very next paragraph opens by immediately resolving it: "To prevent that, a recomputation locks the parent row..." Since the fix follows the problem directly, I'd merge these two into one paragraph rather than splitting a problem from its ready-made solution. Decision 3. The Context above already frames the source of variation as different learning instruments, "any other learning instrument tied to a competency by a competency criterion, such as a course or a rubric criterion." This decision's closing sentence switches to a different axis, competency criteria types, with "completion" as the example. Since supporting completion as a competency criteria type isn't something we're likely to do going forward, I'd swap the example for something in the vein of a course grade or a rubric criterion, consistent with how Context already frames it. |
ADR 0004 is reviewed on its own in openedx#713, which also carries the ADR 0002 and 0003 edits it needs. This PR keeps only ADR 0005. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4 participants
Splits ADR 0004 out of #657 so it can be reviewed on its own. ADR 0005 is not included here.
What's in this PR
docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst, unchanged from Competency Mastery Concurrency ADR #657.Changes to 0002/0003 were filtered from #657, not rewritten. Left behind as ADR-0005-only or unrelated:
user_idforeign keys atsettings.AUTH_USER_MODEL(that comes from ADR 0005's rejected alternative 7)Known gap
ADR 0004 references
:ref:openedx-learning-adr-0005`` in five places, and the 0002/0003 edits reference it in four more. Until ADR 0005 lands, those are undefined labels:sphinxemits nine `ref.ref` warnings. The build still succeeds (`SPHINXOPTS = -W` is commented out in `tox.ini`), but the links are dead. Either 0005 merges soon after this, or the references need rewording.🤖 Generated with Claude Code