From bac8cef6a36143b40d402384787ade02bc03b37b Mon Sep 17 00:00:00 2001 From: rtibblesbot Date: Wed, 30 Sep 2026 21:21:57 -0700 Subject: [PATCH 1/3] fix(qti): validate numeric answers as finite xsd:double values - Replace the loose regex, which accepted `e`, `-`, `1e2e3` and rejected `1e-5`, `1E5` - Compare numeric answers by value, so `21` and `21.0` are duplicates Co-Authored-By: Claude Opus 5.5 (1M context) --- .../textEntry/__tests__/validation.spec.js | 23 +++++++++++++++++++ .../interactions/textEntry/validation.js | 19 ++++++++------- .../QTIEditor/utils/__tests__/math.spec.js | 23 ------------------- .../shared/views/QTIEditor/utils/math.js | 14 +++++++++-- 4 files changed, 44 insertions(+), 35 deletions(-) delete mode 100644 contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/__tests__/math.spec.js diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/validation.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/validation.spec.js index 0f7d905cf4..c91d5738ab 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/validation.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/validation.spec.js @@ -163,6 +163,29 @@ describe('validateTextEntryInteraction', () => { }); }); + const duplicateIds = (values, questionType = QuestionType.NUMERIC) => + validateTextEntryInteraction( + { + ...VALID_NUMERIC_STATE, + answers: values.map((value, i) => ({ id: `a${i}`, value, caseSensitive: false })), + }, + questionType, + ) + .filter(e => e.code === ValidationError.DUPLICATE_ANSWER_CONTENT) + .map(e => e.id); + + it.each([ + ['1e-5', '0.00001'], + ['5', '+5'], + ])('flags %s and %s as duplicates', (first, second) => { + expect(duplicateIds([first, second])).toEqual(['a1']); + }); + + it('does not flag 21 and 21.5 as duplicates', () => { + expect(duplicateIds(['21', '21.5'])).toEqual([]); + }); + }); + describe('valid states return empty array', () => { it('returns [] for a valid numeric state', () => { expect(validateTextEntryInteraction(VALID_NUMERIC_STATE, QuestionType.NUMERIC)).toEqual([]); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/validation.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/validation.js index 457ca0880c..d59e0fd3d0 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/validation.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/validation.js @@ -1,11 +1,11 @@ import { QuestionType, ValidationError } from '../../constants'; -import { floatOrIntRegex } from '../../utils/math'; +import { parseXsdDouble } from '../../utils/math'; import { hasRichTextContent } from '../../utils/richText'; /** * Validate TextEntryState → ValidationError[]. * - * - numeric: prompt required + at least one answer + each value must be a valid number + * - numeric: prompt required + at least one answer + each value a finite xsd:double * - textEntry: prompt required + at least one answer (any non-blank string) * - freeResponse: prompt required only * @@ -30,24 +30,23 @@ export function validateTextEntryInteraction(state, questionType) { for (const answer of answers) { const val = answer.value.trim(); + let lookupKey; if (questionType === QuestionType.NUMERIC) { - if (!floatOrIntRegex.test(val)) { + const number = parseXsdDouble(val); + if (number === null) { errors.push({ code: ValidationError.INVALID_NUMERIC_VALUE, id: answer.id }); } + // Invalid answers keep their text as key so two different ones don't collide. + lookupKey = number ?? val; } else { if (!val) { errors.push({ code: ValidationError.EMPTY_ANSWER_CONTENT, id: answer.id }); } + const normalizedVal = answer.caseSensitive ? val : val.toLowerCase(); + lookupKey = `${normalizedVal}|${answer.caseSensitive}`; } - const normalizedVal = - questionType === QuestionType.TEXT_ENTRY && !answer.caseSensitive ? val.toLowerCase() : val; - const lookupKey = - questionType === QuestionType.TEXT_ENTRY - ? `${normalizedVal}|${answer.caseSensitive}` - : normalizedVal; - if (val) { if (seen.has(lookupKey)) { errors.push({ code: ValidationError.DUPLICATE_ANSWER_CONTENT, id: answer.id }); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/__tests__/math.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/__tests__/math.spec.js deleted file mode 100644 index 346742fa4e..0000000000 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/__tests__/math.spec.js +++ /dev/null @@ -1,23 +0,0 @@ -import { floatOrIntRegex } from '../math'; - -describe('floatOrIntRegex', () => { - it('tests true for valid values', () => { - [ - '1.5', // Float - '-4.5', // Signed Float - '+1', // Signed Int - '10e5', // Exponentiation - '-15.3e5', // Combo - '-12345.67890e98', // Combo 2 - ].forEach(v => expect(floatOrIntRegex.test(v)).toBe(true)); - }); - - it('tests false for invalid values', () => { - [ - 'i * 1.5', // Math - 'one.point.five', // Text - '10 5 0 100', // Spaces - '1.2.3.4', // IP - ].forEach(v => expect(floatOrIntRegex.test(v)).toBe(false)); - }); -}); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/math.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/math.js index 789fe0f90c..fa91058d58 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/math.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/math.js @@ -2,7 +2,17 @@ * Math utilities for the QTI editor. */ +// xsd:double lexical space without INF and NaN. +const xsdDoubleRegex = /^[+-]?(\d+(\.\d*)?|\.\d+)([eE][+-]?\d+)?$/; + /** - * Matches valid numeric answer values: integers, decimals, and scientific notation. + * @param {string} value + * @returns {number|null} the finite number, or null when `value` is not a finite xsd:double */ -export const floatOrIntRegex = /^(?=.)([+-]?([0-9e]*)(\.([0-9e]+))?)$/; +export function parseXsdDouble(value) { + if (!xsdDoubleRegex.test(value)) { + return null; + } + const number = Number(value); + return Number.isFinite(number) ? number : null; +} From 2b0f97af0d81206d8dd81e0f4d6bead45f1ebd27 Mon Sep 17 00:00:00 2001 From: rtibblesbot Date: Wed, 30 Sep 2026 21:22:02 -0700 Subject: [PATCH 2/3] fix(qti): read numeric answers as authored in headless validation Float coercion dropped every answer on an invalid value and merged `21` with `21.0`, so headless validation reported an incomplete question the editor showed no error for. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../QTIEditor/__tests__/validateItem.spec.js | 71 +++++++++++++++++ .../textEntry/__tests__/parse.spec.js | 78 +++++++++++++++++++ .../QTIEditor/interactions/textEntry/parse.js | 61 +++++++++++++-- 3 files changed, 203 insertions(+), 7 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/__tests__/validateItem.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/__tests__/validateItem.spec.js index fa3af04e77..8296090965 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/__tests__/validateItem.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/__tests__/validateItem.spec.js @@ -1,5 +1,8 @@ import { validateItemShape, validateQtiItem } from '../validateItem'; import { QuestionType, ValidationError } from '../constants'; +import { assembleItemXml } from '../serialization/assembleItem'; +import { textEntryInteractionDescriptor } from '../interactions/textEntry/Descriptor'; +import { validateTextEntryInteraction } from '../interactions/textEntry/validation'; import { VALID_CHOICE_ITEM_DOCUMENT, CHOICE_ITEM_DOCUMENT_NO_PROMPT, @@ -64,6 +67,74 @@ describe('validateQtiItem', () => { }); }); + describe('numeric answers', () => { + const { INVALID_NUMERIC_VALUE, DUPLICATE_ANSWER_CONTENT } = ValidationError; + + it.each([ + ...['5', '-5', '+5', '5.', '.5', '1e-5', '1E5', '2.3e+10'].map(value => [[value], []]), + ...[ + 'e', + '-', + '+', + '1e', + '1e2e3', + '1.2.3', + '1,234', + 'INF', + '-INF', + 'NaN', + '1e400', + '0x10', + 'Infinity', + '', + ].map(value => [[value], [INVALID_NUMERIC_VALUE]]), + [['21', '21.0'], [DUPLICATE_ANSWER_CONTENT]], + [['5', '5'], [DUPLICATE_ANSWER_CONTENT]], + [ + ['e', 'e'], + [INVALID_NUMERIC_VALUE, INVALID_NUMERIC_VALUE, DUPLICATE_ANSWER_CONTENT], + ], + [ + ['e', '-'], + [INVALID_NUMERIC_VALUE, INVALID_NUMERIC_VALUE], + ], + ])('reports %j as %j, the same as the editor', (values, expected) => { + const state = { + prompt: '

Enter a number

', + answers: values.map((value, i) => ({ id: `a${i}`, value, caseSensitive: false })), + expectedLength: 0, + }; + const { bodyXml, responseDeclarations } = textEntryInteractionDescriptor.buildXML( + state, + QuestionType.NUMERIC, + ); + const xml = assembleItemXml({ identifier: 'item', title: '', bodyXml, responseDeclarations }); + + expect(codesOf(validateTextEntryInteraction(state, QuestionType.NUMERIC))).toEqual(expected); + expect(codesOf(validateQtiItem(xml))).toEqual(expected); + }); + + it('reports no correct answer past a non-numeric default value', () => { + const { bodyXml } = textEntryInteractionDescriptor.buildXML( + { prompt: '

Enter a number

', answers: [], expectedLength: 0 }, + QuestionType.NUMERIC, + ); + const declaration = + '' + + 'x' + + '5' + + ''; + const xml = assembleItemXml({ + identifier: 'item', + title: '', + bodyXml, + responseDeclarations: [declaration], + }); + + expect(codesOf(validateQtiItem(xml))).toEqual([ValidationError.NO_CORRECT_ANSWER]); + }); + }); + it('reports unparseable XML', () => { expect(validateQtiItem('')).toEqual([ { code: ValidationError.PARSE_ERROR }, diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/parse.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/parse.spec.js index 914caf4c48..9a24c07b62 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/parse.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/parse.spec.js @@ -209,6 +209,84 @@ describe('_extractAnswers', () => { expect(result).toEqual([expect.objectContaining({ value: '', caseSensitive: true })]); }); }); + + describe('numeric values', () => { + it.each(['21.0', '1e-5', 'e', '1e2e3', '1,234'])( + 'reads numeric value %s as authored from and full-credit map-key', + value => { + const declXml = ` + + + ${value} + + + + + + + + `; + expect(_extractAnswers([declXml]).map(a => a.value)).toEqual([value, `${value}9`]); + }, + ); + + it.each(['5.0', ' 5 '])( + 'reads full-credit map-key %j equal to a as one answer', + mapKey => { + const declXml = ` + + + 5 + + + + + + `; + expect(_extractAnswers([declXml]).map(a => a.value)).toEqual(['5']); + }, + ); + + it('reads mapped-value as a leading number, as Mapping.fromXML does', () => { + const declXml = ` + + + 1 + + + + + + `; + expect(_extractAnswers([declXml]).map(a => a.value)).toEqual(['1', '2']); + }); + + it('returns [] for a float declaration with record cardinality', () => { + const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + const declXml = ` + + + 5 + + + `; + expect(_extractAnswers([declXml])).toEqual([]); + expect(errorSpy).toHaveBeenCalled(); + }); + + it.each(['1,234', '1.2.3', '1e400', 'Infinity', '0x10', '', 'NULL'])( + 'reads answers past default value %j, which fromXML accepts', + defaultValue => { + const declXml = ` + + ${defaultValue} + 5 + + `; + expect(_extractAnswers([declXml]).map(a => a.value)).toEqual(['5']); + }, + ); + }); }); describe('parseTextEntryInteraction', () => { diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js index aa1caa08fe..5d196dc661 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js @@ -3,6 +3,7 @@ import { buildXmlNode, parseXML } from '../../serialization/xml'; import CorrectResponse from '../../serialization/qti/declarations/correctResponse'; import Mapping from '../../serialization/qti/declarations/mapping'; import { generateRandomSlug } from '../../utils/generateRandomSlug'; +import { parseXsdDouble } from '../../utils/math'; import { BaseType, QuestionType, RESPONSE_IDENTIFIER } from '../../constants'; const serializer = new XMLSerializer(); @@ -10,9 +11,9 @@ const serializer = new XMLSerializer(); /** * @typedef {object} TextEntryAnswer * @property {string} id - Client-side slug (not serialized to XML) - * @property {string} value - The answer value as a string. For numeric this is a - * float/int string (e.g. "12", "0.5"); for textEntry it - * is a free-form string (e.g. "Paris"). + * @property {string} value - The answer value as a string. For numeric this is the + * authored text, valid or not (e.g. "12", "1e-5"); + * for textEntry it is a free-form string (e.g. "Paris"). * @property {boolean} caseSensitive - textEntry only. When true, "H2O" ≠ "h2o". * Always false for numeric answers. */ @@ -69,6 +70,48 @@ function extractPromptHTML(bodyEl) { return clone.innerHTML.trim(); } +/** + * Numeric answers as authored, read from the XML rather than through + * `QTIDeclaration.fromXML`: its float coercion would throw on an invalid value (dropping + * every answer) or truncate it (`1.2.3` → 1.2), hiding it from validation. + * + * @param {Element} declarationEl - A float `` + * @returns {TextEntryAnswer[]} + */ +function extractNumericAnswers(declarationEl) { + // Built only to validate: throws on a bad identifier or cardinality, as fromXML does. + new QTIDeclaration({ + identifier: declarationEl.getAttribute('identifier'), + baseType: BaseType.FLOAT, + cardinality: declarationEl.getAttribute('cardinality') ?? undefined, + }); + // Run only to throw: fromXML rejects a non-numeric default value, dropping every answer. + for (const el of declarationEl.querySelectorAll(':scope > qti-default-value qti-value')) { + QTIDeclaration.coerceValue(el.textContent.trim(), BaseType.FLOAT); + } + + // Repeats stay, so validation flags them as it does in the editor. + const values = [...declarationEl.querySelectorAll(':scope > qti-correct-response qti-value')].map( + el => el.textContent.trim(), + ); + // Full-credit map-keys are answers too, as on the string path. One equal in value to an + // answer already read (`5.0` for `5`) is that answer, so it is not added again. + const keys = new Set(values.map(value => parseXsdDouble(value) ?? value)); + for (const entry of declarationEl.querySelectorAll(':scope > qti-mapping qti-map-entry')) { + const value = (entry.getAttribute('map-key') ?? '').trim(); + const key = parseXsdDouble(value) ?? value; + if (parseFloat(entry.getAttribute('mapped-value')) >= 1 && !keys.has(key)) { + keys.add(key); + values.push(value); + } + } + return values.map(value => ({ + id: generateRandomSlug('answer'), + value, + caseSensitive: false, + })); +} + /** * Extract correct answer values from the response declaration string. * Returns an array of `{ id, value, caseSensitive }` objects, or [] when no @@ -87,10 +130,15 @@ export function _extractAnswers(responseDeclarations) { if (!declXml) return []; try { - const declaration = QTIDeclaration.fromXML(parseXML(declXml).documentElement); + const declarationEl = parseXML(declXml).documentElement; + if (declarationEl.getAttribute('base-type') === BaseType.FLOAT) { + return extractNumericAnswers(declarationEl); + } + + const declaration = QTIDeclaration.fromXML(declarationEl); const { baseType, correctResponse } = declaration; - if (baseType !== BaseType.FLOAT && baseType !== BaseType.STRING) { + if (baseType !== BaseType.STRING) { // eslint-disable-next-line no-console console.error(`[QTI Editor] Unsupported text-entry base-type: ${baseType}`); return []; @@ -118,8 +166,7 @@ export function _extractAnswers(responseDeclarations) { value, // An answer with no matching qti-map-entry — including every answer in an // item authored before mappings were written — takes the XSD default, false. - // Case sensitivity is a string-only concept, so numeric answers never read it. - caseSensitive: baseType === BaseType.STRING && (caseSensitivity.get(value) ?? false), + caseSensitive: caseSensitivity.get(value) ?? false, })); } catch (err) { // eslint-disable-next-line no-console From 2354019b8df74ba2a3ba3987332fc22b77e18d12 Mon Sep 17 00:00:00 2001 From: rtibblesbot Date: Thu, 1 Oct 2026 07:38:58 -0700 Subject: [PATCH 3/3] fix(qti): flag every repeated Text entry and Numeric answer Co-Authored-By: Claude Opus 5.5 (1M context) --- .../QTIEditor/__tests__/validateItem.spec.js | 17 ++++++++++++--- .../textEntry/__tests__/validation.spec.js | 21 ++++++++++++++----- .../interactions/textEntry/validation.js | 16 ++++++++++---- 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/__tests__/validateItem.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/__tests__/validateItem.spec.js index 8296090965..cb567a6008 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/__tests__/validateItem.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/__tests__/validateItem.spec.js @@ -88,11 +88,22 @@ describe('validateQtiItem', () => { 'Infinity', '', ].map(value => [[value], [INVALID_NUMERIC_VALUE]]), - [['21', '21.0'], [DUPLICATE_ANSWER_CONTENT]], - [['5', '5'], [DUPLICATE_ANSWER_CONTENT]], + [ + ['21', '21.0'], + [DUPLICATE_ANSWER_CONTENT, DUPLICATE_ANSWER_CONTENT], + ], + [ + ['5', '5'], + [DUPLICATE_ANSWER_CONTENT, DUPLICATE_ANSWER_CONTENT], + ], [ ['e', 'e'], - [INVALID_NUMERIC_VALUE, INVALID_NUMERIC_VALUE, DUPLICATE_ANSWER_CONTENT], + [ + INVALID_NUMERIC_VALUE, + INVALID_NUMERIC_VALUE, + DUPLICATE_ANSWER_CONTENT, + DUPLICATE_ANSWER_CONTENT, + ], ], [ ['e', '-'], diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/validation.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/validation.spec.js index c91d5738ab..96fd5874a3 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/validation.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/validation.spec.js @@ -174,11 +174,13 @@ describe('validateTextEntryInteraction', () => { .filter(e => e.code === ValidationError.DUPLICATE_ANSWER_CONTENT) .map(e => e.id); - it.each([ - ['1e-5', '0.00001'], - ['5', '+5'], - ])('flags %s and %s as duplicates', (first, second) => { - expect(duplicateIds([first, second])).toEqual(['a1']); + describe('DUPLICATE_ANSWER_CONTENT (numeric)', () => { + it('flags 1e-5 and 0.00001 as duplicates', () => { + expect(duplicateIds(['1e-5', '0.00001'])).toEqual(['a0', 'a1']); + }); + + it('flags every answer equal in value to another', () => { + expect(duplicateIds(['5', '21', '5.0', '+5'])).toEqual(['a0', 'a2', 'a3']); }); it('does not flag 21 and 21.5 as duplicates', () => { @@ -186,6 +188,15 @@ describe('validateTextEntryInteraction', () => { }); }); + describe('DUPLICATE_ANSWER_CONTENT (textEntry)', () => { + it('flags every case-insensitive answer equal to another', () => { + expect(duplicateIds(['Paris', 'Rome', 'paris'], QuestionType.TEXT_ENTRY)).toEqual([ + 'a0', + 'a2', + ]); + }); + }); + describe('valid states return empty array', () => { it('returns [] for a valid numeric state', () => { expect(validateTextEntryInteraction(VALID_NUMERIC_STATE, QuestionType.NUMERIC)).toEqual([]); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/validation.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/validation.js index d59e0fd3d0..ef0b583439 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/validation.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/validation.js @@ -26,7 +26,9 @@ export function validateTextEntryInteraction(state, questionType) { errors.push({ code: ValidationError.NO_CORRECT_ANSWER }); } - const seen = new Set(); + // A later match flags the first answer too. + const firstSeenId = new Map(); + const duplicateIds = new Set(); for (const answer of answers) { const val = answer.value.trim(); @@ -48,12 +50,18 @@ export function validateTextEntryInteraction(state, questionType) { } if (val) { - if (seen.has(lookupKey)) { - errors.push({ code: ValidationError.DUPLICATE_ANSWER_CONTENT, id: answer.id }); + if (firstSeenId.has(lookupKey)) { + duplicateIds.add(firstSeenId.get(lookupKey)); + duplicateIds.add(answer.id); + } else { + firstSeenId.set(lookupKey, answer.id); } - seen.add(lookupKey); } } + + for (const duplicateId of duplicateIds) { + errors.push({ code: ValidationError.DUPLICATE_ANSWER_CONTENT, id: duplicateId }); + } } return errors;