Repository navigation
fix(scorm): stop writing unresumable progress records; keep quiz scores across sessions - #9
Merged
Merged
Conversation
…es across sessions A learner's attempt failed to load: xttracking_scorm1.2.js threw "Cannot read properties of null (reading 'page_nr')" from setVars, called by initTracking / XTInitialise, and the package never rendered. All four reported problems trace back to how the engine serialises cmi.suspend_data, so they are worked around from the root `script` property (the engine itself is rebuilt from upstream XOT on every release and cannot be patched here). 1. Never write a record that cannot be resumed. getVars() serialises the resume point as find(currentpageid) and pushes the result without checking it; currentpageid is blank between leaving one page and entering the next, and on a revisit to the table of contents, so the saved record contained "interactions":[null]. On the next launch setVars() read page_nr off that null and threw, which unwound the player's setup. The wrapper now drops nulls and substitutes the page on screen (or the last page in the history) as the resume point. Records written by earlier builds are healed on the way in, and initTracking is made non-fatal so a bad record can only cost the resume point, never the whole course. 2. Keep completedPages in sync with the build. It was restored verbatim, so a learner resuming into a build with a different number of tracked pages kept an array of the wrong length - and getSuccessStatus() walks that array, so a short one reports completion early and a long one blocks it forever. It is now resized to state.toCompletePages.length, live tracking options win over saved copies, and page indexes that no longer exist are dropped (which also removes a crash in x_restorePagesViewed). The reported "43 entries against 45 pages" is not a mismatch: 45 counts the injected contents page, 43 excludes it and the unmarked Welcome page. Documented. 3. Completion with trackingMode="full" is now covered by a test that walks all 44 pages, answers all 45 questions and asserts lesson_status=passed with score.raw=100, rather than being assumed. 4. Keep quiz scores across sessions. getVars() persists only the current page's interaction, so every quiz page score was lost on resume and getdRawScore() averaged just the current session - which is how score.raw sits at 0 for a learner with plenty of logged interactions. Scored pages are now persisted compactly (xtPages) and restored as page interactions, with a size guard that keeps the record inside the 4096-character suspend_data limit. The script is no longer maintained by hand inside an XML attribute: source/scorm_progress.js is the source of truth and tools/sync_root_script.py compiles it into data.xml/preview.xml, enforcing the "no < > & or " " rule that keeps the attribute round-tripping through the XOT editor unescaped. Tests: tests/build_scorm_package.sh exports a package with a real XOT container (the release workflow's own import/export path) and tests/test_scorm_tracking.py drives that package in headless Chromium against a mock SCORM 1.2 LMS. 14 tests, all passing here; 6 of them fail on the previous build, including a start-up timeout on the reported record. CI runs both on every PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P9uX9kuo8sErHsfBWBtDyL
The shedding ladder in the getVars wrapper was the one piece of new logic no test exercised. It drops old page history first, then history, then pagesViewed, then remembered quiz scores, so a record that cannot fit still resumes. Drives a learner with 5000 page-history entries: the record stays inside the 4096-character SCORM 1.2 limit, keeps its resume point, all 43 completedPages and all 8 quiz page scores, and resumes with the score intact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P9uX9kuo8sErHsfBWBtDyL
📖 Course content previewDownload the
|
Review of the branch found a case the fix missed, with the same symptom it set out to eliminate. Interactions were checked for nulls but not for whether their page still exists, and XTStartPage hands the saved page_nr straight to the player, which indexes x_pageInfo without a bounds check. A learner whose last save was on a page that a later build removed got "Cannot read properties of null" replaced by "Cannot read properties of undefined (reading 'standalone')" - still a blank course, and reachable in exactly the resize-the-course scenario the completedPages normalisation was written for. Those entries are now dropped too, so the resume point falls back to the contents page. Two smaller things found in the same review: - state.finished is never set by the engine, so the guard against saving after LMSFinish did nothing: onbeforeunload runs XTTerminate, then pagehide autosaved again, writing six values a conformant LMS rejects with error 301 and losing a page-history entry finishTracking had already dropped. Track termination ourselves by wrapping XTTerminate, which has to happen synchronously before xenith assigns it to onbeforeunload. - A save taken mid-page persisted the resume entry as 'entered'. The engine only calls reenter() for an 'exited' interaction, so the entry kept its old start timestamp and the next session's first page exit reported a duration spanning the gap between sessions. The saved copy is now marked exited; the live interaction is untouched. Three new tests, each failing on the previous build, plus the stale completedPages test now builds its record from a scored session so it can assert status rather than only length, and the corrupt-record test asserts the title rendered instead of a character count. 18/18 pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P9uX9kuo8sErHsfBWBtDyL
The comment stripper is a regex, so a /* or */ inside a string literal would silently delete everything up to the next one, and node --check would happily accept the truncated result. Nothing in the current source does that, but the failure mode is invisible in a tool whose whole job is turning reviewable source into shipped code. Assert every function the source defines survives into the compiled output. Verified against the pathological case: a source with '/*' and '*/' held in two string literals now fails the compile instead of shipping a gutted script. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P9uX9kuo8sErHsfBWBtDyL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the four problems reported from a real LMS attempt. All four come from how
xttracking_scorm1.2.jsserialisescmi.suspend_data, so they are worked around from thecourse's root
scriptproperty — the release workflow rebuilds the engine from upstream XOTon every build, so the engine itself cannot be patched here.
1. Course fails to load
getVars()saves the resume point asfind(currentpageid)and pushes the result unchecked(
xttracking_scorm1.2.js:271-273).currentpageidis blank between leaving one page andentering the next (
exitInteraction()clears it at line 483) and on a revisit to the Table ofContents, where
XTEnterPage()is skipped for an already-built page. So the record held"interactions":[null], and the next launch readpage_nroff that null and threw:The exception unwinds the player's setup, so nothing renders — and because the bad value is
stored in
suspend_data, every later launch fails the same way.Two writers produced it: the autosave added in #6 flushed inside that blank window on every
page exit, and the engine's own
finishTracking()does it when a learner saves or closes onthe contents page.
Fix.
state.getVarsis wrapped so an unresumable record is never written — nulls dropped,and when
currentpageiddoes not resolve, the page on screen (or the most recent page in thehistory) is substituted.
state.setVarsis wrapped so records written by earlier builds arehealed on read, and
state.initTrackingis wrapped so an unforeseen bad record can only costthe resume point, never the whole course.
2. Inconsistent saved-progress data
The blank resume point is the same root cause as above.
The reported "43 entries against 45 pages" is correct, not a mismatch.
nrpagesisx_pageInfo.length= 44 content pages + 1 for the contents page Xerte splices in atx_pages[0].completedPagesis sized frommarkedPages, which excludes that page and theWelcome page (
unmarkForCompletion="true", added in #5), giving 43 =page_nr1…43, Welcomethrough the final quiz. "Course complete" (
page_nr44) is deliberately not required. Nowdocumented in
docs/PROJECT_CONTEXT.mdso it does not get "fixed" later.What is a real bug:
setVars()restorescompletedPagesverbatim, so a learner resuminginto a build with a different number of tracked pages keeps an array of the wrong length — and
getSuccessStatus()walks that array, so a short one reports completion early and a long oneblocks completion forever. It is now resized to
state.toCompletePages.length, live trackingoptions (
nrpages, tracking mode, pass mark, page timeout) win over saved copies, and pageindexes that no longer exist are dropped — which also removes a second crash path, since
x_restorePagesViewed()writesx_pageInfo[i].viewedwith no bounds check.3. Completion reachability
Verified rather than assumed.
test_full_walkthrough_reports_passedwalks all 44 pages,answers all 45 questions and asserts
lesson_status="passed"withscore.raw=100;test_completion_survives_a_resumedoes the same across two sessions.For the record, "all pages required" is weaker than it sounds: a page counts as complete when
it is left after more than 100 ms (
exit()ignores shorter visits) and all its interactionsexist — and a quiz page registers all of its question interactions on load. So visiting a quiz
page is enough for completion; answering is what produces the score. The practical risk was
never the page count, it was the crash and the stale array above.
4. Score not captured
getVars()persists only the current page's interaction ("SCORM 1.2 only allows for 4kb ofsuspend data"), and
getdRawScore()averages the page interactions in memory. So every quizpage score was discarded on resume: theme quizzes on Monday and the final quiz on Tuesday
graded as Tuesday alone, and a learner who answers questions without reaching a quiz's results
screen (no
XTSetPageScoreuntil then) reportsscore.raw=0withcmi.interactions.*full ofanswers — the reported combination.
Fix. Scored pages are persisted compactly as
xtPages(
[page_nr, score, weighting, nrinteractions, id]) and restored as page interactions, sogetdRawScore()sees the earlier quizzes again. Becausescoremodeislast, re-taking aquiz still overwrites the remembered score, and no page is counted twice. A size guard keeps
the record inside the 4096-character limit by shedding, in order: old page history, history,
pagesViewed, thenxtPages.Maintainability
The script is no longer maintained by hand inside an XML attribute.
source/scorm_progress.jsis the source of truth andtools/sync_root_script.pycompiles itinto
data.xml/preview.xml(--checkverifies they are in sync; CI and a test both assertit). The compiler also enforces the existing "no
<,>,&or"" rule that keeps theattribute round-tripping through the XOT editor unescaped.
Tests
tests/build_scorm_package.shexports a package with a real XOT container — the sameimport/export path as the release workflow — and
tests/test_scorm_tracking.pydrives thatpackage in headless Chromium against a mock SCORM 1.2 LMS.
.github/workflows/test.ymlruns both on every PR.15/15 pass on this branch, built from a clean clone in a fresh container. 6 of them fail
on
main, including a 45-second start-up timeout on the reported record. The suite covers:loading from the corrupt record, terminate on the contents page, out-of-range
pagesViewed,stale
completedPagesresizing, resume position, the size guard under 5000 history entries,in-session quiz scoring, score retention across a resume, the 75/25 weighting across two
sessions, and the two full-walkthrough completion runs.
Also confirmed in a real browser (Playwright MCP, headed Chrome) against the affected learner's
record:
mainrenders a blank page with that TypeError in the console; this branch renders thecontents with the learner's viewed pages ticked, and a click through into a quiz reports
interactions and a null-free 1098-character record with a valid resume point.
Upstream
The engine bug is unfixed upstream —
thexerteproject/xerteonlinetoolkitsmaster (32c009b)has the identical unguarded code, and there is no issue for it. An issue with a minimal
reproduction (verified against a build of this course with the root
scriptattributestripped, so no code of ours is involved) is drafted and ready to raise. If it is ever fixed
upstream, the autosave half of this script becomes redundant and harmless; the repair half
stays relevant until
getVars()/setVars()are guarded.🤖 Generated with Claude Code
https://claude.ai/code/session_01P9uX9kuo8sErHsfBWBtDyL
Generated by Claude Code