Repository navigation
feat(quiz): hint and explanation on a wrong answer; cover viewed state across a resume - #12
Merged
Merged
Conversation
A learner who picks the wrong option is told only "Your answer is incorrect". Nothing says what they should have been thinking about or where to go and read it, and because the quiz can be restarted with the options reshuffled, they can retake it having learned nothing from the attempt that failed. All 45 questions now carry a hint (what to think about, and which theme to revisit) and an explanation (why the right answer is right, and where it helps, why the tempting distractor is not), shown only when the answer was wrong. The text lives in each question's `feedback` attribute -- the quiz wizard's per-question "General Feedback" field, so the XOT editor exposes it as a normal text area and round-trips it. No hint gives the answer away, and neither field refers to an option by letter or position, because `answerOrder="random"` shuffles them per attempt. models_html5/quiz.html renders that attribute on every submitted answer, right or wrong: after grading it fills three fixed slots in the order the question's `feedbackPos` gives (G the question's own feedback, A the selected option's, C the right/wrong line), defaulting to GAC. Part 6 of source/scorm_progress.js wraps quiz.showFeedBackandTrackResults -- re-wrapped from x_pageLoaded, because the model object is rebuilt for each quiz page -- and empties that slot again when quiz.myProgress[quiz.currentQ] says the answer was right. That is deliberately the failure-safe direction: if upstream renames the hook, the help shows on correct answers too rather than disappearing. The per-option `feedback` field was the alternative and is not used: it says nothing to a learner who fails a Multiple Answer question by under-selecting, and it would duplicate the same paragraph across every wrong option. Option feedback stays empty, so the SCORM interaction records (XTExitInteraction is passed the option feedback) are unchanged. Tests: test_wrong_answer_gets_a_hint_and_an_explanation and test_every_question_carries_wrong_answer_help, plus answer_question() and next_question() helpers -- answer_quiz(correctly=False) ticked the first option, which with shuffled options is not reliably a wrong one. tools/render_preview.py renders the text under each question so reviewers can read all 45 in the PR preview. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0149Y1yzjVXVWwXfsKKeDASd
📖 Course content previewDownload the
|
Review of the first green build, across runtime correctness, content and fidelity to the two OWASP sources, found real problems in all three. Mitigations, checked against the live top10.owasp.org/2025 pages: - Password storage recommended "Argon2 or bcrypt". A04:2025 names Argon2, yescrypt, scrypt and PBKDF2-HMAC-SHA-512, mentions bcrypt only for legacy systems, and leads with the work factor, which we omitted entirely. - Mishandling of exceptional conditions taught "keep sensitive detail out of errors", which is Security Misconfiguration's advice; A10:2025 says to inform the user understandably, and leads with catching errors where they occur, a global handler, full rollback (its term is "failing closed", not "failing secure") and rate limits. We had none of those. - Supply chain recommended "reproducible builds", a word that does not appear on the A03:2025 page, and omitted official sources, deliberately chosen versions and hardened CI/CD. - Insecure design kept "usage limits", a 2021 bullet dropped from A06:2025, and omitted the secure development lifecycle and secure design patterns that page now leads with. Taxonomy and question quality: - Broken access control was described as "a person exceeding permissions". OWASP says users, never human-only, which is the distinction this course exists to teach for agents and non-human identities. - Two adjacent final-quiz hints stated opposite classification rules, by harm and by root cause, so a learner applying either consistently failed the other. Both now point at the disambiguator in the question's own stem. - One hint asked whether provenance is any use "once it is already running", which argues for False on a question keyed True. - One hint said the agent "did exactly what its target rewarded", leaking the correct option "Rogue Agents (reward hacking)" by word stem. - One sentence reproduced OWASP's own ASI01 phrasing closely enough to read as copied, which the course's own-words rule does not allow; plus a lower-case acronym expansion and an imprecise "direct object reference". Runtime, traced against the engine in models_html5/quiz.html: - helpSlot treated an empty feedbackPos as GAC, but the engine spreads the value it finds, so an empty one leaves it no slot letters and puts the right/wrong line in all three. On a correct answer we would have deleted the verdict. It now follows the engine's rule: only an absent attribute means GAC. - Where the panel is centred vertically the model slides feedback in, and jQuery captures the target height while the text is still present, so emptying afterwards animated a blank gap open and snapped it shut. The slot is now cleared through jQuery, ending the animation first. Latent today (no page sets verticalAlign) but it would surface the moment one did. - The header comment and PROJECT_CONTEXT claimed the hook was failure-safe in general. That holds for a renamed call, not for a change of slot order. Both now say which. Tests, two of which could pass while the feature was broken: - The every-question walk asserted only that its "missing" list was empty, so it would have passed if the walk matched no nodes at all. It now asserts it inspected all 45. - "correctFeedback" is a Python substring of "incorrectFeedback", so that assertion held whatever the grading outcome. It is now an exact match, and the correct-answer case also asserts the verdict line survived the clear. - The wrong-answer test now continues onto a second quiz page, which is the case the re-wrap from x_pageLoaded exists for and nothing covered. Also corrects a pre-existing stale count in PROJECT_CONTEXT (the 45 questions are 27 theme + 18 final, not "23 theme + 18 final + 4"), in both the prose and the verification checklist, and records the carve-out the new text needs from the per-page acronym convention: wrong-answer help is read on its own, so each acronym is expanded within the hint and within the explanation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0149Y1yzjVXVWwXfsKKeDASd
The new documentation had the tells of machine-written text: bolded mini
headings chopping short prose into labelled chunks, em-dash asides stacked two
to a sentence, a rule-of-three summary line, and one bullet carrying four
separate corrections in a single semicolon-joined sentence. The conventions
were also headed with slogans ("Every question teaches on failure") where a
plain name does the job.
Rewritten as prose. No technical claim, count, file path, attribute name or
test name changes; the README bullet now says what a correct answer shows
instead of leaving it as an implied contrast.
The 45 learner-facing strings were checked against the same criteria and left
alone: they carry no stock contrast constructions, no inflated vocabulary and
no em-dashes, and their three-item lists enumerate real OWASP controls rather
than padding a cadence.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0149Y1yzjVXVWwXfsKKeDASd
Reported: save the activity, leave, come back, and the header progress bar reads 0% with the contents-page ticks gone. Both read x_pageInfo[i].viewed, which is session state. It survives a resume only through the engine's pagesViewed field: x_pagesViewed() -> state.pagesViewed -> cmi.suspend_data -> setVars -> x_restorePagesViewed(). Nothing covered that round trip. test_header_progress_bar_counts_viewed_pages never resumes, and test_resume_returns_to_the_saved_page only checks which page comes back, so a break anywhere along the chain shipped unnoticed. This test walks three pages, terminates, and asserts pagesViewed is in the saved record with those pages in it, then resumes and asserts the viewed flags, the progress bar and the ticks all come back. The halves are separate so a failure says whether the record was written wrong or read back wrong. Reading the code did not find the fault: persist() sets state.pagesViewed before saving, repairOutgoing keeps it, repairIncoming filters it against the live page list, and x_pageInfo is built in x_projectDataLoaded well before setVars runs. Two candidates remain that the test will discriminate. fitBudget sheds pagesViewed second, before quiz scores, so a record over the 4000 character budget loses the ticks first. And persist() sets cmi.core.exit to '' whenever the status is not incomplete, which some LMSs treat as attempt-over and answer with empty suspend data on the next launch. This environment has no Docker, so CI is the only executor: the test is pushed to reproduce, not because it is known to pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0149Y1yzjVXVWwXfsKKeDASd
RichardoC
changed the title
The first resume test passes: after three pages, pagesViewed is written and restored, and the ticks and bar come back. So the round trip is sound and the reproduction was too gentle. fitBudget sheds pagesViewed second when the record will not fit the 4000 character suspend_data budget, ahead of the quiz scores, which makes the ticks and the progress bar the first learner-visible thing to go as a record grows. A learner saves after a real session, not after three pages. This walks pages 1 to 35, answering the quizzes on the way, then saves and resumes. It asserts the record still fits, that pagesViewed survived, and that the flags, ticks and bar come back, with the record size in the failure message so a red run says how far over budget it went. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0149Y1yzjVXVWwXfsKKeDASd
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
2 participants
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.
Two threads: the wrong-answer help, and a reported bug where a learner who saves, leaves and comes back finds the progress bar empty and the ticks gone.
Wrong-answer help
A learner who picks the wrong option is told only "Your answer is incorrect", then the quiz moves on. With the options reshuffled on restart, they can retake it having learned nothing from the failed attempt.
All 45 questions now carry a hint (what to think about, which theme to revisit) and an explanation (why the right answer is right, and where it helps, why the tempting distractor is not), shown only when the answer was wrong. No hint gives the answer away, and neither field names an option by letter or position, since
answerOrder="random"shuffles them.The text sits in each question's
feedbackattribute, the quiz wizard's per-question General Feedback field, so the XOT editor round-trips it. The engine renders that attribute after every submitted answer, right or wrong, into whichever of three slots the question'sfeedbackPosgives toG(defaultGAC, so#topFeedback). Part 6 ofsource/scorm_progress.jswrapsquiz.showFeedBackandTrackResults, re-wrapped fromx_pageLoadedbecause the model object is rebuilt per quiz page, and empties that slot whenquiz.myProgress[quiz.currentQ]says the answer was right.Per-option
feedbackwas the alternative and is unused: it says nothing to someone who fails a Multiple Answer question by under-selecting, and would repeat the same paragraph on every wrong option. Tracking is untouched, becauseXTExitInteractionis handed the option feedback, which is still empty everywhere.Tests:
test_wrong_answer_gets_a_hint_and_an_explanation(both shown when wrong, neither when right, across two quiz pages) andtest_every_question_carries_wrong_answer_help(asserts it inspected all 45).render_preview.pyrenders the text under each question for review.Review pass (
31c95cd)Four mitigations were out of date against the live
top10.owasp.org/2025pages, each verified before changing: password storage named bcrypt, which A04 now reserves for legacy systems, and omitted the work factor; mishandling of exceptional conditions carried Security Misconfiguration's advice about hiding error detail, where A10 leads with catching at source, a global handler, full rollback and rate limits; supply chain recommended reproducible builds, a phrase absent from A03; insecure design kept "usage limits", which A06 has dropped.Also corrected: broken access control was described as human-only, where OWASP says users, which is the point for agent identities; two adjacent final-quiz hints stated opposite classification rules; one hint argued against its own keyed answer; one leaked its answer by word stem.
Two latent runtime defects.
helpSlottreated an emptyfeedbackPosasGAC, but the engine leaves no slot letters in that case, so the wrapper would have deleted the verdict line. And clearing after the model's slide-in animation left a blank block animating open, so the slot is now cleared through jQuery.Two tests could pass while the feature was broken: the every-question walk asserted only that its failure list was empty, so a walk matching nothing passed, and
"correctFeedback"is a Python substring of"incorrectFeedback".Viewed state across a resume (
f12c05e,4a5b9d6)Reported against the released package: save the activity, leave, come back, and the header progress bar reads 0% with the contents-page ticks gone.
Both read
x_pageInfo[i].viewed, which is session state. It survives a resume only through the engine'spagesViewedfield:x_pagesViewed()→state.pagesViewed→cmi.suspend_data→setVars→x_restorePagesViewed(). Nothing covered that round trip, which is how a break in it could ship:test_header_progress_bar_counts_viewed_pagesnever resumes, andtest_resume_returns_to_the_saved_pageonly checks which page comes back.Two tests now cover it, and both pass, so the bug is not reproduced here.
test_viewed_pages_survive_a_save_and_resumewalks three pages, saves and resumes.test_viewed_pages_survive_a_resume_late_in_the_coursewalks pages 1 to 35 answering the quizzes on the way, becausefitBudgetshedspagesViewedsecond when a record will not fit the 4000 character budget, ahead of the quiz scores, which would make the ticks the first learner-visible casualty of a record that has grown. At that size the record still fits,pagesViewedsurvives, and the flags, ticks and bar all come back. Both halves are asserted separately so a future failure says whether the record was written wrong or read back wrong.So the two candidates that could be tested here are both ruled out: the round trip is sound, and a realistic record does not overflow. That leaves the difference between the mock LMS and a real one. The leading remaining candidate is
cmi.core.exit:persist()sets it to'suspend'only while the status isincomplete, and to''otherwise (source/scorm_progress.js:406), matching stockfinishTracking. An LMS is entitled to read''as attempt-over and start the next launch with empty suspend data, which would lose the viewed flags along with everything else. Confirming that needs the values the LMS actually holds, not another mock run.The tests stay either way as the regression cover that was missing.
Verification
Locally: sync check,
node --checkon the compiled script, both XML files parse and stay byte-identical, all 45 carry feedback in the existing double-escaped shape, every tracking attribute count unchanged.No Docker here, so CI is the convention-9 gate. It builds the package in a real XOT container and runs the tracking suite, green at 26/26 on this head. Before merge it is still worth confirming a Publish round-trip preserves the new
feedbackvalues, and walking a quiz inplay.phpto check the layout at narrow widths.Pre-existing, not changed
Two scenarios repeat near-verbatim between a theme quiz and the final quiz; three final-quiz questions have two options where the rest have four; "reproducible builds" still appears on two content pages, which is reasonable practice, just not A03 guidance.
🤖 Generated with Claude Code
https://claude.ai/code/session_0149Y1yzjVXVWwXfsKKeDASd