Skip to content

refactor(tracking): retire the xAPI grading channel, SCORM 1.2 is the only one - #124

Merged
erseco merged 18 commits into
mainfrom
refactor/remove-xapi-grading-channel
Aug 21, 2026
Merged

refactor(tracking): retire the xAPI grading channel, SCORM 1.2 is the only one#124
erseco merged 18 commits into
mainfrom
refactor/remove-xapi-grading-channel

Conversation

@erseco

@erseco erseco commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Replaces #122, which went the other way — it tried to make the xAPI overall work. Measuring it end to end showed that is not worth doing.

Summary

The plugin grades through two channels. They are not equivalent, and the one enabled by default is the worse of the two.

SCORM 1.2 xAPI
Per-iDevice grade 100 / 0 identical
Weighted overall (25/75) 25 50, or nothing at all
Attempts, completion, gradebook, events full the same code

The xAPI ingestor calls into track::apply_item_scores() and attempts::record_item() for everything except the overall. So the channel duplicates transport, authentication, validation and identity resolution to reach the same code, and the one thing it does on its own is the one thing it gets wrong.

It was always experimental

No product requirement asked for it — DEC-0-14 answered "no por ahora" to whether there was demand for LRS analytics. Its scope explicitly excluded cmi5 and any external LRS (DEC-17-01 §6, DEC-0-18 §9). This plugin has been its only consumer.

The numbers

Measured end to end in a real Moodle: same package, same learner actions, two pages, weights 25 / 75, first answered fully correct, second fully wrong. The weighted overall is 25.

  • SCORM publishes 25, in both PERITEM and OVERALL grade models.
  • xAPI publishes 50. statement_normalizer hardcodes 'weighted' => 0.0, so recompute_overall_pct() falls into its unweighted-mean branch.
  • On main today a multipage xAPI attempt gets no overall row at all — no gradebook column, no attempt_completed, no OVERALL grade model.

Weights cannot travel on this channel: they are not in the statements, and putting them there was rejected on principle — package structure supplied by a learner's browser must not reshape another learner's grade.

What is NOT lost

  • iDevice coverage. There is one dispatch site in the exported package and both channels share its trigger; the SCORM half only additionally needs pipwerks, which this plugin injects into every served page. No gradable iDevice reaches one channel and not the other.
  • Identity across pages. This was xAPI's one real advantage — stable object IRIs against SCORM's page-local slots. The versioned exe12/ payload closes it: records carry their own stable id.
  • Attempts, completion, gradebook, events, mobile. Shared code, untouched. Mobile was SCORM-only anyway; xAPI never had a web service.

exelearning_tracking_events is dropped, public release and all

This section changed twice, so here is the whole reasoning rather than just the conclusion.

An intermediate revision dropped the table claiming the plugin had never been published. That premise was false, and review caught it:

$ git show v4.0.2:db/install.xml | grep -c exelearning_tracking_events
1

v4.0.3 returns the same, and xapi_track.php is in both trees, so the published releases ship the table and a working writer. A site that used the xAPI channel can hold rows, and the table is absent from backup/moodle2. That correction stands.

The table is dropped anyway — a deliberate call, not an oversight. Three reasons:

  • What is lost is audit, not assessment. Grades, attempts and reports live in exelearning_grade_item and exelearning_attempt and are untouched. This table held per-statement audit metadata — statementid, verb, objectid, registration, scaled, user and timestamp — and the statement.id deduplication those ids provided. It never stored the statement JSON, and no grade calculation has ever read it.
  • The exposure window is very short. v4.0.2 is from 2026-07-07, the channel was switchable off, and the plugin is in early deployment. The row volume in the field is negligible against carrying an inert table indefinitely.
  • Keeping it was not free. Neither exelearning_delete_instance() nor exelearning_reset_userdata() ever cleaned it, so deleting an activity or resetting a course orphaned learner-linked rows — unreachably, since the privacy API finds them by joining through the {exelearning} row that had just been deleted. Retaining an inert table meant fixing and then maintaining two cleanup paths forever, for a log nothing reads.

CHANGELOG.md announces it under Removed, so an administrator who wants those rows can dump them before upgrading.

So the definition leaves db/install.xml, all six sites leave classes/privacy/provider.php (metadata, the context-list and users-in-context queries, the export path and all four delete paths), and the eight language strings that translated them go with it.

Stage 19, which created the table, is left byte-identical — upgrade history is append-only. A new stage 21 drops it where it exists, and also calls

unset_config('xapiprimaryenabled', 'exelearning');

which is the pattern stage 20 already uses for the three configs the editor installer maintained. The setting was removed from settings.php, from the language files and from its helper, but its stored value was not: a site that had switched the channel on kept an orphan row in mdl_config_plugins that a future setting reusing the name would inherit. Now nothing survives the retirement.

Verified in a real Moodle, upgrading from the pre-stage version with the table populated and the setting on:

BEFORE: table=present rows=1 xapiprimaryenabled='1'
AFTER:  table=absent xapiprimaryenabled=false

A second stage (22, 2026082101) adds the gradable column described below. scripts/check-version.sh (DEC-111-01, wired into CI) requires $plugin->version to be strictly greater than every savepoint, so the version is 2026082102.

One deliberate call worth reviewing

disableTracking is removed, not repointed at gradeenabled.

Silencing the client for ungraded activities looks tidy but breaks two things: classes/completion/custom_completion.php queries exelearning_attempt for the itemnumber = 0 row with no grading filter, and mod_form.php::add_completion_rules() does not gate that rule on gradeenabled — so a teacher can require completion-by-status on an ungraded activity. It would also empty the attempt history DEC-13-07 promises can be recalculated when grading is re-enabled.

disableTracking was born with this channel and nothing would ever set it true again, so removing it restores the shim to its pre-DEC-85-01 behaviour.

Now fixed in this PR — see Turning grading off did not stop grading below. It was briefly attempted in #105 as an early return in track::ingest(), which broke completion by status; that commit has been dropped from #105 and the correct version lands here.

Upgrading with a page open

Upgrading while a learner has an xAPI-era page open leaves that tab with an inert SCORM shim — disableTracking was baked into the served page — and a listener posting to an endpoint that no longer exists. Its bounded retry (2 attempts, 3 s / 6 s) then drops those statements, so interactions in that tab are lost until the page reloads.

A maintenance-mode consideration rather than a data-loss path: attempts already committed are unaffected, and there is no retry storm. Recorded in DEC-122-01 and docs/tracking-architecture.md.

Language strings

A sweep of lang/ for xapi|lrs|statement|tincan now returns nothing, and no orphan is left behind: the eight strings that translated the dropped table's privacy declarations go with it, and the orphan viewstub string — which advertised an "iframe + sidebar + xAPI bridge" that does not exist — is removed from all five files. No language file still names the retired channel.

Separately, gradeenabled_help promised "no grade columns, no reports and no attempt tracking". The last third is false, and has been since the switch shipped: DEC-13-07 chose "ocultar y conservar" precisely so attempts survive. exelearning_sync_grade_items() removes the grade items; nothing touches exelearning_attempt. The clause is deleted — the diff is a pure removal — and the help text now ends at "no grade columns and no reports".

It belongs in this PR because removing disableTracking rests on the same fact: attempts must keep being written, since completion by status reads the itemnumber = 0 row with no grading filter.

Turning grading off did not stop grading

This is the defect this PR is most responsible for, because it is this PR that makes it reachable.

view.php passed disableTracking = $emitsxapi (DEC-85-01), so the SCORM shim was inert for any package that emits xAPI — which is every recent eXeLearning export, since the emitter ships in all of them. Removing disableTracking makes the shim always live, and a defect that existed in main only for non-emitting packages becomes universal.

track::ingest() never consulted gradeenabled. With the switch off the instance has no grade items, so the registered-objectid filter empties itemscores, the server-side recompute never runs, and the OVERALL publication falls back to the CLIENT's cmi.core.score.raw. And grade_update() recreates a deleted grade item — so the effect is not writing into an existing column, it is resurrecting the one the teacher just removed:

after switching grading OFF : overall=NO ITEM   items=0
after a learner submits     : overall=95.00000  attempts=1

That directly contradicts the help text this same PR ships: "no grade columns and no reports".

The guard goes at the single point that publishes, beside the grade-model condition already there:

if ($grademodel === EXELEARNING_GRADEMODEL_OVERALL && !empty($exe->gradeenabled)) {

Not as an early return in ingest(). The attempt row has to keep being written: DEC-69-01's completion by status reads it filtering on exelearningid, userid, itemnumber and status — never on gradeenabled — and mod_form.php does not gate that rule on the switch, so completion by status is settable, and has to work, on an ungraded activity.

apply_one()'s per-iDevice grade_update() needs no guard, and that is measured rather than argued. The test runs both grade models; the peritem case is green before the fix and stays green, because remove_all() marks the rows deleted = 1 and no objectid is left to recognise. A second check there would be dead code implying a path that does not exist. Recorded in DEC-124-02.

The test asserts $grades->items === [], which is stronger than "the value is null" — it requires the grade_item itself not to reappear.

And what about work done while it was off?

The two sections above compose into a question neither answers, and the answer used to depend on the grade model — which is the worst possible answer.

With grading off no objectid is registered, because remove_all() marks the mappings deleted = 1. So:

  • PERITEM wrote no itemnumber > 0 rows at all. Nothing to resurrect.
  • OVERALL wrote the itemnumber = 0 row completion needs, carrying a score that had not been through the server-side recompute — there was nothing to recompute from. It came straight from the browser's cmi.core.score.raw, and re-enabling published it.

Reproduced, same sequence in both models:

OVERALL  Item 0 must have no grade derived from the ungraded interval
         Failed asserting that '95.00000' is null.
PERITEM  (passes)

The same work by the same learner became a mark or not depending on a setting that says nothing about that — and where it did, the mark came from an unverified client value.

The decision: option A

The alternative was to keep per-iDevice scores through the off period and suppress only grade_update(), so re-enabling recovered everything with a proper server-side overall. Rejected: it requires objectids registered while the switch is off — exactly what remove_all() undoes — and it turns the switch into a pause button, where work done "outside assessment" ends up assessed anyway.

So the switch is a statement about what the activity is. A row written with grading off is completion-only, and the aggregation ignores it, always:

  • new gradable column (INT, NOTNULL, DEFAULT 1), added by upgrade stage 22 (2026082101). Existing rows default to gradable — everything recorded before this stage came from an ingest() that made no such distinction, and assuming gradable is the conservative choice: it preserves grades a site already published;
  • attempts::record_item() takes $gradable (default true, so no other caller changes) and track::ingest() passes !empty($exe->gradeenabled) at both write sites. The one inside apply_one() is unreachable with grading off, but passes the value rather than hardcoding true so the two cannot drift apart;
  • all three aggregation queries filter gradable = 1aggregate_scaled(), fetch_scaled_by_user_item(), and the participation summary's mean. One unfiltered query would bring the asymmetry back through it.

Nothing is lost. The row keeps its score, status and timestamps, feeds completion by status and appears in the attempts report. It just never counts towards a grade. The field is declared to the privacy API, exported with transform::yesno(), and carries its string in five languages.

Which queries filter, and which must not. It is not a rule by category — it is one decision per query, and DEC-124-03 carries the table:

query filters why
aggregate_scaled(), fetch_scaled_by_user_item(), the summary's mean yes they are the grade
count_user_attempts(), behind maxattempt yes maxattempt is a grading control; charging it for work the activity declared outside assessment leaves a learner at the cap having never had a gradable attempt
the cap check in ingest() does not run while grading is off same reason: an ungraded activity must not refuse a learner on the strength of graded attempts spent earlier
MAX(attempt) in resolve_attempt_number() no it allocates the next number; skipping rows would reissue an existing one and collide with record_item()'s upsert key
participation COUNT(DISTINCT userid) no it counts who participated, not who was graded — and since the mean does filter, the summary now returns graded too and the string names both populations
custom_completion no completion is the reason the row exists

The flag belongs to the attempt, not the row — and the invariant is that every row of an attempt shares it. A session keeps the attempt it started with even if the switch flips underneath it; a write inherits completion-only if any row of the attempt already is, and the first ungraded write takes every row of the attempt down with it. Demoting only the row being written leaves a mixed attempt, which breaks three things at once: count_user_attempts() counts an attempt as used if any row is gradable, so it keeps charging maxattempt; the surviving gradable row can be republished when grading returns; and the inheritance itself becomes order-dependent, since IGNORE_MULTIPLE has no ORDER BY. Splitting the session into a second, gradable attempt was tried and is wrong: the client accumulates its itemScores map and never clears it — deliberately, so a failed POST cannot lose a score — so a fresh gradable attempt is a clean vessel for contaminated content. Measured against the split:

attempt=1 item=0 raw=95 gradable=0
attempt=2 item=0 raw=95 gradable=1
attempt=2 item=1 raw=95 gradable=1
GRADE item 1 = '95.00000'

A session that crossed the switch produces no trustworthy grade, so it produces none. Reloading mints a new token and a clean attempt, at no cost to the learner. The accepted price is that they keep working ungraded until they reload, with no signal — preferable to turning ungraded work into a mark, and their work is still recorded, still counts for completion, and still costs them no attempts. A UI signal needs the client to read a server flag and is noted as follow-up.

Both grade models had their own fallback to the client's score when there is no gradable history — $score in ingest()'s overall, $rawitem in apply_one(). Closing only the first left PERITEM publishing the browser's 95 beside rows correctly marked gradable = 0. Both now require gradable history to publish, which is why the mid-session test runs both models.

Two known limitations, both deliberate: the report's download gains a "counts towards grade" column, but the on-screen table still lists these rows with their score unmarked; and rows recorded before this version stay gradable, since after the fact there is no reliable way to tell which were written while the switch was off — a backfill keyed on "instances ungraded right now" does not discriminate, because switching off soft-deletes every mapping, so it would demote legitimate history on any site currently switched off.

test_work_done_while_ungraded_never_becomes_a_grade runs both models: the learner completes the activity with the switch off, the rows are asserted gradable = 0, the teacher enables grading, and every recreated column must be empty while the itemnumber = 0 row survives. Verified red in two variants to separate the halves — without the flag both models fail the gradable assertion; with the flag written but the readers unfiltered, overall fails on '95.00000' is null and peritem passes, which is the asymmetry itself. Stage 22 verified against an existing install (BEFORE: absentAFTER: present). Recorded in DEC-124-03.

Re-enabling "Graded activity" lost the grades — fixed here

Found while checking the help string above, and it turned out to be real. DEC-13-07 keeps exelearning_attempt when the switch goes off precisely so that "reactivar gradeenabled re-detecta y recalcula desde el historial", and grade_item_manager::remove_all() repeats it in its own docblock. Only the re-detect half happened.

STEP 1 graded on  : overall='80.00000'  attempts=2
STEP 2 graded off : overall='NO ITEM'   attempts=2
STEP 3 re-enabled : overall='NULL'      attempts=2   ← column back, empty
STEP 4 after an explicit exelearning_update_grades(): overall='80.00000'

grade_sync::sync() recreates the gradebook columns but never repopulates them. The republish-from-history call added in DEC-34-01 (B2) lives in exelearning_update_instance(), and its condition enumerated grademodel and grademethodgradeenabled was not in it. A teacher who switched grading off and back on got empty columns for learners already assessed, with the data sitting in exelearning_attempt the whole time.

gradeenabled joins that condition, and the $oldrow SELECT that feeds it. Two details worth review:

  • The off direction needs no guard of its own. grade_sync::update_grades() already returns immediately when gradeenabled is unset, so nothing is published into the items sync() has just deleted. A direction check here would be dead code.
  • When the caller omits the field, fall back to the stored value, not to a constant. grademodel/grademethod have a safe default earlier in the function; gradeenabled does not, and assuming enabled would let a programmatic update switch grading on in silence. With the fallback, omitting the field means unchanged.
  • $data is hydrated with that fallback, which is not cosmetic. exelearning_update_grades() returns early on an empty gradeenabled, so a programmatic caller that changed grademodel/grademethod without passing gradeenabled reached the republish call and had it silently do nothing — pre-existing, and now covered:
B0 PERITEM item1 = 80.00000
B1 isset(data->gradeenabled) before = false
B2 stored gradeenabled = '1' | OVERALL column = NULL
$newgradeenabled = (int) ($data->gradeenabled ?? $oldrow->gradeenabled);
if (
    (int) $data->grademodel !== (int) $oldrow->grademodel
    || (int) $data->grademethod !== (int) $oldrow->grademethod
    || $newgradeenabled !== (int) $oldrow->gradeenabled
) {
    exelearning_update_grades($data, 0);
}

test_gradeenabled_toggled_back_on_republishes_from_history walks the whole cycle with a real attempt ingested through track::ingest(). Verified red without the fix — "Failed asserting that null matches expected 80.0" — and green with it. DEC-124-01 records the rule the condition now expresses: any pure grading-configuration change republishes from history, a content change deliberately does not (DEC-12-01 snapshot-and-warn). DEC-13-07 stays Accepted and unedited — this makes its promise true rather than changing it.

Alternative considered

Leaving xapiprimaryenabled at 0 and keeping the code. The kill switch does work — xapi_track.php refuses to grade when it is off — so that would also make SCORM authoritative. It was rejected: the dead code stays, the endpoint stays published, and the decision stays untaken. Recorded in DEC-122-01 §Opciones.

This does not close the door on xAPI

The emitter lives on upstream — every exported package keeps emitting statements. What is retired here is the consumer, which is the easy half to rebuild; the hard half, speaking xAPI, is untouched.

If a real analytics requirement appears, the starting point is not this grading channel but a core_xapi handler for events, beside the grade pipeline instead of competing with it. DEC-122-01 records what would have to be solved first — chiefly the per-iDevice weight, which does not travel in the statements and without which a correct overall cannot exist.

Tests

phpunit (full suite)     →  332 tests, 1209 assertions, exit 0
npx vitest run           →  21 tests passed
composer lint (phpcs)    →  exit 0, 118 files
make architecture-check  →  exit 0, 68 records
scripts/check-version.sh →  exit 0

Surviving tracking_events references

All intentional: db/upgrade.php stage 19 (append-only history) and the new drop stage; emit_tracking_events() in track.php and tracking_events() in its test helper, which are method names unrelated to the table; the prose recording the removal; and the append-only evidence archive under research/. No live production-code reference to the table remains.

Merge order

#125 first, then this. Both branch from the same main and both touch js/scorm_tracker.js and its spec, but in different places — #125 rewrites captureItemScores(), this removes disableTracking — so they are complementary, not conflicting. The ordering is a matter of meaning rather than mechanics: this PR makes SCORM the only grading channel, so the silent multipage loss #125 fixes should be gone before that happens.

Relationship with core

eXeLearning #2302 reduces the emitter to an analytics feed with no package verdict. The two are independent: this plugin ignores the emitter either way, and old packages that still emit are harmless — the emitter posts fire-and-forget inside a try, so a message nobody listens for is discarded by the browser.

Decision recorded in DEC-122-01, superseding DEC-85-01, DEC-17-01 and DEC-0-18.

… only one

The xAPI ingestion channel was always experimental. No product requirement asked
for it — DEC-0-14 answered "not for now" to whether there was demand for LRS
analytics — its scope explicitly excluded cmi5 and any external LRS, and this
plugin has been its only consumer.

With both channels in production the result could finally be measured end to end
rather than reasoned about, and the measurement does not support keeping it.

The real problem is duplication: two channels grading the same thing, with two
transports, two authentications, two validations and two identity resolutions,
feeding one grade pipeline. That is only worth paying for if the second path can
do something the first cannot.

It cannot.

- Per-iDevice grading is IDENTICAL. One dispatch site in the exported package
  fires both pipwerks and the xAPI track, so the channels are coextensive:
  neither captures a score the other misses.
- The overall was worse. Per-iDevice `answered` statements carry no weight, so
  the channel had to take the overall from a package statement. Measured on a
  25/75 pair: SCORM recomposes the correct weighted mean server-side (25) while
  the xAPI derivation yields an unweighted 50. The one thing the channel did on
  its own is the one thing it got wrong.
- Everything else was already shared. The ingestor CALLS into
  track::apply_item_scores() and attempts::record_item(); attempts, completion,
  gradebook publication and events came from the same code either way.

So the channel bought one extra public endpoint, an inline listener, a full
normaliser, an admin setting and an inert mode for the SCORM shim, for a measured
benefit of zero.

`exelearning_tracking_events` is KEPT, inert: it holds learner-linked rows
declared to the privacy API and is absent from backup/moodle2, so dropping it
would destroy personal data with no restore path.

`disableTracking` is removed rather than repointed at `gradeenabled`. It was born
with this channel and nothing would set it true again. Silencing the client for
ungraded activities would break completion-by-status, which queries
exelearning_attempt with no grading filter, and would empty the attempt history
DEC-13-07 promises can be recalculated when grading is re-enabled.

Old packages keep emitting and that is harmless: the emitter posts fire-and-forget
inside a try, so a message nobody listens for is discarded by the browser.

This does not close the door on xAPI. It retires a redundant second grading path;
the emitter itself lives on upstream, and a future analytics integration would be
a different and better-posed thing — a core_xapi handler for events, beside the
grade pipeline instead of competing with it.

Decision recorded in DEC-122-01, superseding DEC-85-01, DEC-17-01 and DEC-0-18.
@codecov-commenter

codecov-commenter commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.50746% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 93.82%. Comparing base (85f5b99) to head (bd9b1bd).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
classes/local/attempts.php 95.23% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #124      +/-   ##
============================================
+ Coverage     92.32%   93.82%   +1.49%     
+ Complexity      824      738      -86     
============================================
  Files            51       47       -4     
  Lines          3740     3350     -390     
============================================
- Hits           3453     3143     -310     
+ Misses          287      207      -80     
Flag Coverage Δ
javascript 94.11% <ø> (-0.23%) ⬇️
php 93.81% <98.50%> (+1.57%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
PHP (server-side) 93.81% <98.50%> (+1.57%) ⬆️
JavaScript (SCORM tracker) 94.11% <ø> (-0.23%) ⬇️
Files with missing lines Coverage Δ
classes/external/get_user_attempts.php 100.00% <100.00%> (ø)
classes/grades/grade_item_manager.php 98.82% <ø> (ø)
classes/local/package_manager.php 97.48% <ø> (+1.03%) ⬆️
classes/local/track.php 96.96% <100.00%> (+0.12%) ⬆️
classes/local/tracking_endpoint.php 100.00% <100.00%> (ø)
classes/privacy/provider.php 94.02% <100.00%> (+2.21%) ⬆️
js/scorm_tracker.js 94.11% <ø> (-0.12%) ⬇️
lib.php 82.45% <100.00%> (+2.31%) ⬆️
classes/local/attempts.php 96.75% <95.23%> (-0.33%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

erseco added 8 commits August 21, 2026 10:05
…ning it

Review found that retaining the table was both unnecessary and not wired
correctly, so it goes.

Unnecessary: the first pass kept it on the grounds that it holds learner-linked
rows declared to the privacy API with no restore path, since it is absent from
backup/moodle2. That reasoning assumed sites in the field. The plugin has never
been published, so no site holds that table or any row in it.

Not wired correctly: neither exelearning_delete_instance() nor
exelearning_reset_userdata() ever cleaned it. Deleting an activity or resetting a
course left learner-linked rows behind, and the privacy API locates them by
joining through the instance that had just been deleted. Keeping the table would
have meant fixing both paths first — carrying a latent orphan-data bug for a
channel nobody writes to any more.

So: the table definition leaves db/install.xml, and every one of its six sites in
classes/privacy/provider.php goes with it (metadata, the context-list and
users-in-context queries, the export path, and all four delete paths), along with
its eight language strings. What remains declares exactly the tables that exist.

Stage 19, which created the table, is left byte-identical: upgrade history is
append-only and has to keep working for a site that installed a dev build. A new
stage 21 drops the table where it exists, so such a site creates it on the way
through and drops it immediately after. Verified by re-initialising the PHPUnit
environment, whose upgrade step ran the drop against a database that had the
table.

The version bump could not reuse the savepoint number: scripts/check-version.sh
(DEC-111-01, wired into CI) requires $plugin->version to be STRICTLY greater than
every savepoint. Hence savepoint 2026082100 and version 2026082101.

Also removes the orphan `viewstub` string from all five language files. Nothing
referenced it, and it advertised an "iframe + sidebar + xAPI bridge" that does not
exist. No language string still promises xAPI behaviour.

Adds an operational note to DEC-122-01 and the tracking doc: upgrading while a
learner has an xAPI-era page open leaves that tab with an inert SCORM shim and a
listener posting to an endpoint that is gone. Its bounded retry then drops those
statements, so interactions are lost until the page reloads. A maintenance-mode
consideration, not a data-loss path — committed attempts are unaffected.
…erly

This reverses the previous commit's decision. That commit dropped the table on
the premise that "the plugin has never been published, so no site holds that
table or any row in it". The premise is false.

v4.0.2 (7 Jul 2026) and v4.0.3 are public releases. Both ship the table in
db/install.xml AND a working xapi_track.php that writes to it, and the README
tells people to install from the Releases ZIPs:

    $ git show v4.0.2:db/install.xml | grep -c exelearning_tracking_events
    1

So sites in the field can hold rows, and dropping the table in an upgrade would
be a destructive migration of real learner-linked data — audit data, but
personal data all the same, and absent from backup/moodle2, so with no restore
path. That is not something to do as a side effect of retiring a channel.

The table is therefore restored to its state in f28bd4b, along with every one of
its six sites in classes/privacy/provider.php (metadata, the context-list and
users-in-context queries, the export path, all four delete paths) and its eight
language strings. The new stage 21 that dropped it is removed entirely: the
append-only rule protects RELEASED upgrade history, and that stage exists only
on this unmerged branch, so it must not ship. Stage 19 stays untouched as
always. The version still bumps (2026082100) for the strings and code, but no
longer has to clear a new savepoint.

What the review got right stands: retention was never wired correctly. Neither
exelearning_delete_instance() nor exelearning_reset_userdata() cleaned the
table, so deleting an activity or resetting a course left learner-linked rows
behind — and out of reach, since the privacy API locates them by joining through
{exelearning}, the very instance that had just been deleted. Both paths now
delete its rows, delete_instance() doing so before the instance row goes.

That gap is fixed here rather than left latent, which is what made dropping the
table look attractive in the first place. Covered by tests in the files that
already cover each function; both fail without the lib.php change, each on the
seeded row surviving:

    1) lib_helpers_test::test_reset_userdata_deletes_attempts
    Failed asserting that 1 is identical to 0.
    2) lib_test::test_delete_instance
    Failed asserting that 1 is identical to 0.

The table stays inert with a view to being removed in a future release, once
there is a migration story for the rows existing sites already hold — exporting,
archiving or purging them with notice. DEC-122-01, the tracking doc, status.yaml
and the CHANGELOG record the correction and that intent.

viewstub is NOT restored: removing that orphan string was correct and stays.
The operational note about upgrading with a page open is unaffected and kept.

Validated: full PHPUnit suite (326 tests, 0 failures), npx vitest run,
composer lint, make architecture-check, scripts/check-version.sh.
…cked

gradeenabled_help promised "no grade columns, no reports and no attempt
tracking". The last third is false, and has been since the switch shipped:
DEC-13-07 chose "ocultar y conservar" precisely so that attempts survive,
which is what makes the toggle reversible — re-enabling it recalculates
from the preserved history. exelearning_sync_grade_items() removes the
grade items; nothing touches exelearning_attempt.

The string now says what actually happens, and says why it matters. It
belongs in this PR: removing disableTracking rests on the same fact —
attempts must keep being written, because completion by status reads that
row with no grading filter.

Five languages; the non-English ones keep their ~ untranslated marker.
…and all

Reverses the retention decided two commits ago, and records the real
reason this time instead of a false one.

The retention rested on review evidence that the drop's premise was
wrong: the earlier commit claimed the plugin had never been published,
and it had —

    $ git show v4.0.2:db/install.xml | grep -c exelearning_tracking_events
    1

v4.0.3 returns the same, and xapi_track.php is in both trees, so the
published releases ship the table and a writer that fills it. That
correction stands. What changes is the conclusion drawn from it.

The table goes anyway, deliberately, and DEC-122-01 §4 now says so in
those terms rather than pretending nothing is lost:

- what is lost is audit, not assessment. Grades, attempts and reports
  live in exelearning_grade_item and exelearning_attempt and are
  untouched; this table held the raw statement and its statement.id
  deduplication;
- the exposure window is very short. v4.0.2 is from 2026-07-07, the
  channel was switchable off, and the plugin is in early deployment, so
  the row volume in the field is negligible against carrying the table
  indefinitely;
- keeping it was not free. Neither exelearning_delete_instance() nor
  exelearning_reset_userdata() ever cleaned it, so deleting an activity
  or resetting a course orphaned learner-linked rows — unreachably, as
  the privacy API finds them by joining through the {exelearning} row
  just deleted. An inert table meant fixing and then maintaining two
  cleanup paths forever, for a log nothing reads.

CHANGELOG announces it under Removed, so an administrator who wants
those rows can dump them before upgrading.

So the definition leaves db/install.xml, all six sites leave
classes/privacy/provider.php (metadata, context list, users in context,
export, four delete paths) and the eight language strings that
translated them go with it. Stage 19 stays byte-identical — upgrade
history is append-only — and stage 21 (2026082100) drops the table where
it exists. Verified by creating the table, inserting a row and running
xmldb_exelearning_upgrade(2026072400): present with 1 row before, absent
after. version.php goes to 2026082101, strictly above the savepoint, as
scripts/check-version.sh (DEC-111-01) requires.

A sweep of lang/ for xapi|lrs|statement|tincan now returns nothing, and
no orphan string is left behind: no language file still names the
retired channel. The surviving tree hits are emit_tracking_events() and
its test helper — method names unrelated to the table — the append-only
research/ archive, and the prose recording this removal.

phpunit 326/1179 exit 0 · vitest 21/21 · phpcs 0 · architecture-check 0
· check-version 0
…e log raw

Two review points on the retirement, both correct.

The setting was removed from settings.php, from the lang files and from
its helper, but its stored value was not. Stage 21 now calls

    unset_config('xapiprimaryenabled', 'exelearning');

which is exactly the pattern stage 20 already uses for the three configs
the editor installer maintained. A site that had switched the channel on
kept an orphan row in mdl_config_plugins that a future setting reusing
the name would inherit; now nothing survives the retirement. DEC-122-01
said the orphan was harmless and left it — that bullet is corrected
rather than left standing.

The other point: the CHANGELOG, the ADR and the tracking doc all said
the drop discards "raw statements". It never held one. The table stored
statementid, verb, objectid, registration, scaled, userid and
timecreated — audit metadata, verified against the shipped definition:

    $ git show v4.0.2:db/install.xml | sed -n '/tracking_events/,/<\/TABLE>/p'

All three now say audit/idempotency metadata, name the fields, and state
explicitly that the statement JSON was never stored. Overstating what a
destructive migration destroys is as much a documentation defect as
understating it.

Stage 21's comment also still described "a site that installed a dev
build", which is the story the ADR has already corrected: v4.0.2 and
v4.0.3 are public releases that shipped the table with a writer. The
comment now says so, and carries the trade-off inline so a reader of
upgrade.php does not have to reach the ADR to learn the migration is
knowingly destructive.

Verified in a real Moodle, upgrading from the pre-stage version with the
table populated and the setting on:

    BEFORE: table=present rows=1 xapiprimaryenabled='1'
    AFTER:  table=absent xapiprimaryenabled=false

phpunit 326/1179 exit 0 · vitest 21/21 · phpcs 0 · architecture-check 0
· check-version 0
…estating it

The previous commit replaced "no attempt tracking" with a claim that
attempts keep being recorded "so re-enabling recalculates from that
history". The first half is true. The second half is not, and I had
taken it from DEC-13-07 rather than from running it.

Measured, toggling the switch off and back on with a graded attempt in
place:

    STEP 1 graded on  : overall='80.00000'  attempts=2
    STEP 2 graded off : overall='NO ITEM'   attempts=2
    STEP 3 re-enabled : overall='NULL'      attempts=2
    STEP 4 after an explicit exelearning_update_grades(): overall='80.00000'

grade_sync::sync() recreates the gradebook columns but never repopulates
them, and the republish-from-history call in exelearning_update_instance()
is gated on grademodel/grademethod having changed — gradeenabled is not
in that condition. So the history survives and is recoverable, but
nothing recovers it automatically. DEC-13-07's "recalcula desde el
historial" is aspirational for this transition; tracked separately.

Rather than describe any of that in a form tooltip, the sentence is gone:
the help text now ends at "no grade columns and no reports". The diff
against main is a pure deletion of the false clause, which is all this
string needed.

phpunit 326/1179 exit 0 · phpcs 0
DEC-13-07 keeps exelearning_attempt when the master grading switch goes
off precisely so that "reactivar gradeenabled re-detecta y recalcula
desde el historial", and grade_item_manager::remove_all() says the same
in its own docblock. Only the re-detect half happened.

    STEP 1 graded on  : overall='80.00000'  attempts=2
    STEP 2 graded off : overall='NO ITEM'   attempts=2
    STEP 3 re-enabled : overall='NULL'      attempts=2
    STEP 4 after an explicit exelearning_update_grades(): overall='80.00000'

grade_sync::sync() recreates the gradebook columns but never repopulates
them. The republish-from-history call added in DEC-34-01 (B2) lives in
exelearning_update_instance(), and its condition enumerated grademodel
and grademethod — gradeenabled was not in it. So a teacher who switched
grading off and back on got empty columns for learners who had already
been assessed, with the data sitting in exelearning_attempt the whole
time.

gradeenabled joins that condition, and the $oldrow SELECT that feeds it.
Two details:

- the off direction needs no guard of its own. grade_sync::update_grades()
  already returns immediately when gradeenabled is unset, so nothing is
  published into the items sync() has just deleted. A direction check
  here would be dead code;
- when the caller omits the field, fall back to the STORED value rather
  than to a constant. grademodel/grademethod have a safe default earlier
  in the function; gradeenabled does not, and assuming "enabled" would
  let a programmatic update switch grading on in silence. With the
  fallback, omitting the field means "unchanged", which is correct.

test_gradeenabled_toggled_back_on_republishes_from_history walks the
whole cycle with a real attempt ingested through track::ingest: grade,
switch off (asserting the column goes and the attempts do not), switch
back on, require the 80 back. Verified red without the fix — "Failed
asserting that null matches expected 80.0" — and green with it.

DEC-124-01 records the rule the condition now expresses: any pure
grading-configuration change republishes from history, while a content
change deliberately does not (DEC-12-01 snapshot-and-warn). DEC-13-07
stays Accepted and unedited; this makes its promise true rather than
changing it. Both copies of the remove_all() docblock now name where the
recompute half actually happens, so nobody assumes sync() does it.

phpunit 327/1187 exit 0 · vitest 21/21 · phpcs 0 · architecture-check 0
(66 records) · check-version 0
…not ingestion

Retiring the xAPI channel removes disableTracking, and view.php used to
pass disableTracking = $emitsxapi (DEC-85-01) — so the SCORM shim was
inert for any package that emits xAPI, which is every recent eXeLearning
export. With the shim always live, a defect that already existed in main
for non-emitting packages becomes universal, and this PR is what makes it
reachable. So it is fixed here.

track::ingest() never consulted gradeenabled. With the switch off the
instance has no grade items, so the registered-objectid filter empties
itemscores, the server-side recompute never runs, and the OVERALL
publication falls back to the CLIENT's cmi.core.score.raw. And
grade_update() RECREATES a deleted grade item, so the effect is not
writing into an existing column but resurrecting the one the teacher just
removed:

    after switching grading OFF : overall=NO ITEM   items=0
    after a learner submits     : overall=95.00000  attempts=1

That contradicts the form's own help text — "no grade columns and no
reports" — which this same PR ships.

The guard goes at the single point that publishes, beside the grade-model
condition already there:

    if ($grademodel === EXELEARNING_GRADEMODEL_OVERALL && !empty($exe->gradeenabled)) {

NOT as an early return in ingest(). The attempt row has to keep being
written: DEC-69-01's completion by status reads it filtering on
exelearningid, userid, itemnumber and status — never on gradeenabled —
and mod_form.php does not gate that rule on the switch, so completion by
status is settable, and must work, on an ungraded activity. It is also
the history DEC-13-07 preserves and DEC-124-01 now recomputes from.

apply_one()'s per-iDevice grade_update() needs no guard, and that is
measured rather than argued: the test's peritem case is GREEN before the
fix and stays green after, because remove_all() marks the rows deleted=1
and no objectid is left to recognise. A second check there would be dead
code implying a path that does not exist.

Also fixes a pre-existing hole in the republish path this PR touched
earlier. exelearning_update_grades() returns early on an empty
gradeenabled, so a programmatic caller that changed grademodel or
grademethod without passing gradeenabled reached the republish call and
had it silently do nothing:

    B0 PERITEM item1 = 80.00000
    B1 isset(data->gradeenabled) before = false
    B2 stored gradeenabled = '1' | OVERALL column = NULL

$data is now hydrated from the stored value, so an omitted field means
"unchanged" for the comparison AND for the call. The form was never
affected; it posts the whole object.

Both tests verified red first — "two arrays are identical" for the guard,
"null matches expected 80.0" for the hydration — and DEC-124-02 records
the boundary. DEC-13-07 stays Accepted and unedited.

phpunit 330/1196 exit 0 · vitest 21/21 · phpcs 0 · architecture-check 0
(67 records) · check-version 0
erseco added 9 commits August 21, 2026 12:28
…r model

DEC-124-02 established that the 'Graded activity' switch silences
publication, not ingestion: the attempt row keeps being written because
completion by status needs it. DEC-124-01 made re-enabling the switch
republish from history. Composed, they left a question unanswered — what
happens to what the learner did WHILE the switch was off — and the answer
depended on the grade model, which is the worst possible answer.

With grading off no objectid is registered, because remove_all() marks
the mappings deleted = 1. So PERITEM wrote no itemnumber>0 rows and had
nothing to resurrect, while OVERALL wrote the itemnumber=0 row completion
needs — carrying a score that had NOT been through the server-side
recompute, since there was nothing to recompute from. It came straight
from the browser's cmi.core.score.raw, and re-enabling published it:

    OVERALL  Item 0 must have no grade derived from the ungraded interval
             Failed asserting that '95.00000' is null.
    PERITEM  (passes)

The same work by the same learner became a mark or not depending on a
setting that says nothing about that. And where it did, the mark came
from an unverified client value.

The alternative was to keep per-iDevice scores through the off period and
suppress only grade_update(), so re-enabling recovered everything with a
server-side overall. Rejected: it needs objectids registered while the
switch is off — exactly what remove_all() undoes — and it makes the
switch a pause button, where work done "outside assessment" ends up
assessed anyway.

So the switch becomes a statement about what the activity IS. A row
written with grading off is marked completion-only and the aggregation
ignores it, always:

- new gradable column (INT, NOTNULL, DEFAULT 1), added by upgrade stage
  22 (2026082101). Existing rows default to 1 — everything recorded
  before this stage was written by an ingest() that made no such
  distinction, and assuming gradable preserves grades a site already
  published;
- attempts::record_item() takes $gradable (default true, so no other
  caller changes) and track::ingest() passes !empty($exe->gradeenabled)
  at BOTH write sites. The one inside apply_one() is unreachable with
  grading off, but passes the value rather than hardcoding true so the
  two cannot drift apart;
- all THREE aggregation queries filter gradable = 1: aggregate_scaled(),
  fetch_scaled_by_user_item(), and the participation summary's mean. One
  unfiltered query would bring the asymmetry back through it.

Nothing is lost: the row keeps its score, status and timestamps, feeds
completion by status and appears in the attempts report. It just never
counts towards a grade. The field is declared to the privacy API,
exported with transform::yesno(), and has its string in five languages.

Verified red in two variants to separate the halves of the fix. Without
the flag, both models fail the gradable assertion; with the flag written
but the readers unfiltered, overall fails on "'95.00000' is null" and
peritem passes — the asymmetry itself. Stage 22 verified against an
existing install: BEFORE gradable=absent, AFTER gradable=present.

DEC-124-03 records the decision.

phpunit 332/1209 exit 0 · vitest 21/21 · phpcs 0 · architecture-check 0
(68 records) · check-version 0
The Moodle PHPDoc Checker fails a function whose @PARAM list does not
match its signature, and adding $gradable without its @PARAM broke all
22 matrix jobs with a single error repeated:

    Line 231: Phpdocs for function attempts::record_item has incomplete
    parameters list (error)

composer lint runs phpcs only; the PHPDoc Checker is a separate
moodle-plugin-ci step with no local equivalent here, which is what let it
through. Swept every PHP file this branch touches for the same mismatch
between documented and actual parameters — this was the only one.
…s and maxattempt

Three review findings on DEC-124-03, all confirmed, plus one the review
did not reach.

P1, blocker — gradable did not survive backup/restore.
backup_exelearning_stepslib.php omitted it from the attempt element and
restore inserts the rows straight into the table, so a completion-only
row came back with the column default: gradable=1. A backup/restore could
silently convert work done while the activity was NOT an assessment into
gradable history, which the next switch-on would publish as a mark. The
field is now carried, and restore reads it explicitly with a fallback to
1 for backups taken before this decision — same reasoning as the upgrade
step. This is the identical defect already recorded for gradeenabled
itself (B4, DEC-34-01), one column later.

P2 — a mid-session flip rewrote the flag. record_item() re-applied
$gradable on update, and the upsert is keyed by the session's attempt
number, so a learner holding a page open while the teacher flips the
switch had their row rewritten. The client makes that certain rather than
unlikely: scorm_tracker.js accumulates itemScores and never clears the
map (deliberately, so a failed POST cannot lose a score), so every later
POST re-sends everything captured while grading was off.

Gradability is now decided when the row is created and never revisited.
Not "sticky zero": re-deciding breaks BOTH directions — off->on promotes
work done outside assessment, and on->off would demote work that was done
under assessment, destroying the history DEC-124-01 promises to recover.
Splitting the attempt on a state change was the other candidate and is
rejected in the ADR: resolve_attempt_number() keys on sessiontoken, so
splitting means one token mapping to two attempt numbers, and reloading
already gives a clean attempt — which now costs the learner nothing.

P2 — maxattempt was consumed by ungraded work, and here the ADR was
simply wrong. It claimed the counting queries deliberately do not filter.
maxattempt is a grading control (mod_form disables it with the rest of
the grade settings when the activity is ungraded), so charging it for
work the activity itself declared outside assessment produces a state
with no way out: at maxattempt=1 the learner reaches the limit having
never had a gradable attempt and can never be graded. count_user_attempts()
now filters. MAX(attempt) in resolve_attempt_number() still must not —
skipping rows there reissues an existing attempt number and collides with
record_item()'s upsert key. The ADR now says which query filters and why,
one row at a time, instead of a rule by category.

And the one the review did not reach, found because the mid-session test
failed on the grade rather than on the flag: with no gradable history at
all, aggregate_scaled() returns null and the code fell back to $score —
the client's cmi.core.score.raw. It published an unverified browser value
in exactly the case where the server had decided none of the history
counts. Publishing the overall now requires gradable history. The
ordinary first POST is unaffected: record_item() runs first, so with
grading on there is always a gradable row by then.

Three tests, each verified red on its own failure:

  preserves_gradable_off               Failed asserting that 1 is identical to 0
  mid_session_does_not_promote         Failed asserting that 1 is identical to 0
  ungraded_attempts_do_not_consume_max Failed asserting that false is true

The backup test seeds one gradable and one completion-only attempt, so it
proves the value round-trips rather than that everything lands on one
constant.

phpunit 335/1221 exit 0 · vitest 23/23 · phpcs 0 · architecture-check 0
(69 records) · check-version 0
…everywhere it is read

A parallel audit of the gradable flag found the previous commit's own
reasoning was wrong, and three surfaces it had not reached.

The correction first. That commit justified "never re-apply gradable on
update" by saying a demotion would destroy history DEC-124-01 promises to
recover. The premise is false: the four lines below the flag already
REPLACE rawscore, maxscore, scaledscore and status with the incoming
POST's values, because the upsert keeps one row per (attempt, itemnumber)
rather than appending. The prior history was gone either way, and only
the flag survived — attached to content it no longer described.

So the rule is now the other one: the flag can be lowered but never
raised. A write carrying ungraded-period data takes the row down with it,
because the flag has to describe the score currently stored. Raising it
is what must never happen; that is the guarantee the flag exists to make.

And the situation mostly stops arising, because resolve_attempt_number()
now matches the session on gradability as well as on sessiontoken, so a
session that straddles a switch change is SPLIT into two attempts. This
is what the review asked for, and it fixes a harm the previous design
accepted too easily: with a single row, a learner whose teacher enabled
grading mid-session completed the whole activity into a row that could
never produce a mark, silently. Now they get a fresh gradable attempt on
the next autocommit — which costs them nothing, since count_user_attempts()
does not charge the ungraded one against maxattempt. Each attempt is then
homogeneous, which is what lets the flag describe the scores beside it.

Three surfaces the flag had not reached:

- view.php counted the learner's attempts itself instead of asking
  count_user_attempts(), so after that function started filtering, the
  displayed "1 of 1 used" contradicted a server that would still accept
  the attempt. It now calls the same function that enforces the cap. This
  divergence was introduced by the previous commit.
- get_user_attempts, a public contract registered for the mobile service,
  returned completion-only attempts with a score and no way to tell them
  apart, next to maxattempt — the exact ingredients for a client to
  render both a phantom grade and a wrong cap. It now returns 'gradable'
  per attempt and 'usedattempts', the count the server actually enforces.
  The list stays unfiltered: a learner's history is their history.
- participation_summary() computed its mean over gradable rows but its
  count over all of them, so one rendered sentence described two
  populations. It now returns 'graded' too and the string names both.

The report's download gains a "counts towards grade" column, because a
CSV that cannot be reconciled against the gradebook is worse than no
export. The on-screen table is still unmarked and stays a follow-up.

Docs corrected where this branch had invalidated them: GRADEBOOK.md's
"when grading is disabled" section, TRACKING.md's aggregation and cap
rules, PRIVACY_BACKUP_FILES.md's field table, EXTERNAL_SERVICES.md, and
the generated ADR index.

The CHANGELOG now says the guarantee applies to work recorded from this
version on. Stage 22 deliberately does not backfill: "instances ungraded
right now" does not identify ungraded-period rows, because switching off
soft-deletes every mapping, so a backfill would demote legitimate history
on any site currently switched off.

phpunit 337/1233 exit 0 · vitest 23/23 · phpcs 0 · architecture-check 0
(69 records) · check-version 0 · phpdoc sweep over 27 changed files clean
…session

Both review findings confirmed, and the first one shows the previous
commit's split was the wrong answer.

Splitting a session into a second, gradable attempt when the switch flips
on gave the re-sent client scores a clean vessel to land in. The client
accumulates its itemScores map and never clears it — deliberately, so a
failed POST cannot lose a score — so every later POST re-sends everything
captured during the ungraded period, and the server cannot tell which
entries were earned before the switch and which after. Reproduced against
the split:

    attempt=1 item=0 raw=95 gradable=0
    attempt=2 item=0 raw=95 gradable=1
    attempt=2 item=1 raw=95 gradable=1
    GRADE item 1 = '95.00000'

So the session keeps the attempt it started with, and the flag belongs to
the ATTEMPT rather than the row: record_item() takes the minimum of what
the write carries and what the attempt already holds. That closes the
hole the split had left open in apply_one(), which inserts a per-iDevice
row mid-attempt and would otherwise insert it gradable.

A session that crossed the switch produces no trustworthy grade, so it
produces none. Reloading mints a new token and a clean attempt, which
costs the learner nothing because count_user_attempts() does not charge
the ungraded one against maxattempt. The accepted price — they keep
working ungraded until they reload, with no signal — is preferable to
turning work done outside assessment into a mark, and their work is not
lost: recorded, counted for completion, and free of charge against the
cap. Surfacing it in the UI needs a server signal the client reads, and
is noted as follow-up.

Second finding: the cap's $sessionknown escape hatch let a session opened
during the ungraded period carry its exemption into a later gradable
attempt. Fixed at the root rather than by adding gradable to that lookup:
the cap now only runs while the activity is graded. maxattempt is a
grading control, and with count_user_attempts() filtering, leaving it
armed also refused a learner access to an UNGRADED activity on the
strength of graded attempts they spent earlier.

And one the review did not reach, found because the repro still published
after the flag was correct: apply_one() has its own fallback to the
CLIENT's $rawitem when aggregate_scaled() returns null, mirroring the one
already closed on the overall. Closing only the overall left PERITEM
publishing the browser's 95 next to rows correctly marked gradable=0.
Both paths now require gradable history to publish, and the mid-session
test runs both grade models for exactly that reason.

Both new tests verified red against the split:

  mid_session_produces_no_grade          Failed asserting that 2 is identical to 1
  ungraded_session_cannot_win_an_extra   Failed asserting that false is true

They send the SAME accumulated itemscores map on both POSTs, which is
what the client really does and what the previous test omitted.

Also: participation_summary()'s @return shape now lists graded.

phpunit 339/1245 exit 0 · vitest 23/23 · phpcs 0 · architecture-check 0
(69 records) · check-version 0 · phpdoc sweep clean
…tten

The invariant the ADR states — the flag belongs to the ATTEMPT — was not
what the code implemented. record_item() resolved it with

    $DB->get_field(..., ['attempt' => $attempt], IGNORE_MULTIPLE)

and then wrote it to the one row it was upserting. IGNORE_MULTIPLE has no
ORDER BY, so that is min(incoming, an arbitrary row of the attempt), not
min(incoming, the attempt) — and a mixed attempt was reachable, which is
what makes the difference matter.

Reproduced in BOTH grade models, not just PERITEM as reported: the
per-iDevice row survives at gradable=1 while the overall row goes to 0,
because the ungraded POST rewrites only the overall — with the mappings
soft-deleted the objectid filter empties itemscores, so apply_one() never
runs. In OVERALL apply_one() still WRITES that row, it just does not
publish it, so the mixed state forms there too.

    itemnumber 1 must have been taken down with the attempt
    Failed asserting that 1 is identical to 0.   (overall AND peritem)

A mixed attempt breaks three things at once. count_user_attempts() counts
an attempt as used if ANY of its rows is gradable, so it keeps charging
maxattempt — reintroducing an asymmetry between grade models, which is
exactly what this decision exists to remove. The surviving gradable row
can be republished when grading returns. And it corrupts the inheritance
itself, since the next write asks the attempt a question whose answer
depends on row order.

So the rule now runs over the whole attempt in both directions: a write
inherits completion-only if any row of the attempt already is, and the
first ungraded write set_field()s every row of the attempt down. The
guard around it means the extra write only happens on the transition, not
on every autocommit of an ungraded session.

The test covers both models and the full ON -> OFF -> ON cycle with the
same session token and the same accumulated itemscores map, asserting
every row of the attempt, that count_user_attempts() drops to 0, and that
switching back on republishes nothing.

Also: participation_summary()'s empty branch now returns 'graded' => 0,
so the function satisfies the @return shape it declares.

phpunit 340/1260 exit 0 · vitest 23/23 · phpcs 0 · architecture-check 0
(69 records) · check-version 0 · phpdoc sweep clean
@erseco
erseco merged commit d454fcc into main Aug 21, 2026
25 checks passed
@erseco
erseco deleted the refactor/remove-xapi-grading-channel branch August 21, 2026 13:52
erseco added a commit that referenced this pull request Aug 21, 2026
main retired the xAPI grading channel (#124), so this branch's xAPI half
goes with it. Eight conflicts, resolved as follows.

Deleted, accepting main's removal: js/xapi_listener.js and its Vitest
spec, docs/xapi-integration-plan.md, docs/xapi-qa-checklist.md.

view.php: kept this branch's secure-bridge narrative and dropped the
channel arbitration built around it — $emitsxapi, the xapi_config()
block, the listener injection and the disableTracking argument, which
scorm_config() no longer takes. The secure/legacy split, the bridge
relay, the watchdog notice and the external-media host are untouched.

js/scorm_bridge_relay.js: removed its disableTracking support. It existed
so the relay could stay live while forwarding no score for an
xAPI-primary package; with no xAPI channel, scorm_config() never emits
the key and the branch was dead code. Its Vitest case went with it.

README.md and docs/tracking-architecture.md take main's account of the
retirement; the secure-mode prose this branch adds is preserved where it
does not describe xAPI. research/docs/indices/adrs.yaml is generated, so
it was regenerated rather than hand-merged.

vitest 122/122 · phpcs 0 · architecture-check 0 (76 records)
erseco added a commit that referenced this pull request Aug 21, 2026
…time writes

The vendored runtime this PR replaces is only half the story. The plugin
supplies its copy ONLY to packages that do not carry one —
package_manager.php:254, `if ($present) { continue; }` — and eXeLearning's
SCORM exporter DOES carry one: Scorm12Exporter.spec.ts asserts the
assembled file ships all five layers, client, activities, policy,
lifecycle, adapter. The activities layer is the one that writes
cmi.suspend_data as `exe12/1|{record}|{record}…`.

So the moment core#2209 ships, every SCORM export uploaded here arrives
with a runtime writing a payload neither of this plugin's parsers can
read — `grep -c exe12` was 0 in both classes/local/track.php and
js/scorm_tracker.js — and every per-iDevice grade in those packages is
lost silently. That is caused by this change, not independent of it, so
it belongs in this PR rather than in one of its own.

Both parsers now dispatch on the header and nothing downstream re-sniffs
the payload. One representation serves both formats: every entry carries
title, scorepct and weighted, plus an `objectid` key on — and only on —
an entry that knows its own identity. A versioned record needs no DOM
resolution and cannot collide, so captureItemScores() and the server's
legacy itemnumber fallback both step aside for it; everything else there
exists only because the legacy format cannot name its owner and reuses
the page-local slot N across pages (DEC-5-01).

Version handling is deliberately strict. An unreadable version tag, or
one newer than the parser understands, yields an EMPTY map rather than a
best-effort parse: a future revision may reorder fields, and a silently
misparsed one would publish a WRONG grade — worse than publishing none,
which merely leaves the item visibly ungraded.

Three record shapes are skipped on purpose, each documented where it is
skipped: three-field records (migrated-but-unclaimed legacy entries that
name a page position, not an iDevice), records without the evaluable flag
(the producer excludes them from cmi.core.score.raw, so they must not
reach a gradebook column either), and records whose score field is empty
(no result yet, which is not the same as scoring 0).

Ported from the trace-replay branch rather than copied: that branch
predates #124 and #125, so the exe12 half was lifted onto the current
files and the legacy attribution work it also carries was left there.
filter_registered_scores() is extracted from the inline filter in
ingest(), because the objectid map recovered from suspend_data needs the
same protection against injected objectids.

Verified red without the parser: disabling the dispatch fails all four
new PHP tests and nothing else.

phpunit 344/1287 exit 0 · vitest 33/33 · phpcs 0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request high priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants