diff --git a/client/src/lib/constants.js b/client/src/lib/constants.js index ee99778..68d8818 100644 --- a/client/src/lib/constants.js +++ b/client/src/lib/constants.js @@ -35,3 +35,9 @@ export const BLOOM_LEVELS = [ // Mirrors MAX_MATERIALS_PER_OBJECTIVE in src/constants/app-constants.js. // The server rejects writes above this; the UI stops the instructor first. export const MAX_MATERIALS_PER_OBJECTIVE = 3; + +// Mirrors MAX_QUESTIONS_PER_OBJECTIVE in src/constants/app-constants.js — see +// there for why this is the only cap. It bounds one GRANULAR objective's total. +// The server clamps to it; the steppers disable at it so the instructor sees the +// limit rather than having it silently applied. +export const MAX_QUESTIONS_PER_OBJECTIVE = 20; diff --git a/client/src/lib/questionTypes.js b/client/src/lib/questionTypes.js new file mode 100644 index 0000000..a2c28d8 --- /dev/null +++ b/client/src/lib/questionTypes.js @@ -0,0 +1,125 @@ +// The question-type breakdown is the single source of truth for how many +// questions a granular objective generates, and for which Bloom levels it +// covers. Every derivation lives here so the card, the chip badges, the type +// panel, and the generation request cannot disagree about the same array — +// they previously did, one reading it with find() and the rest with reduce(). +// +// Shape: [{ bloomLevel, questionType, count }] + +import { + QUESTION_TYPES, + DEFAULT_BLOOM_TYPE_PREFERENCES, + MAX_QUESTIONS_PER_OBJECTIVE, +} from "./constants"; + +/** Total questions across every Bloom level and type. */ +export function totalQuestions(questionTypes) { + return (questionTypes || []).reduce((sum, qt) => sum + (qt.count || 0), 0); +} + +/** Total questions for one Bloom level, across all its types. */ +export function levelTotal(questionTypes, bloomLevel) { + return (questionTypes || []) + .filter((qt) => qt.bloomLevel === bloomLevel) + .reduce((sum, qt) => sum + (qt.count || 0), 0); +} + +/** Questions for one (level, type) pair. Sums rather than taking the first + * match, so a duplicated pair agrees with the totals above. */ +export function pairCount(questionTypes, bloomLevel, questionType) { + return (questionTypes || []) + .filter((qt) => qt.bloomLevel === bloomLevel && qt.questionType === questionType) + .reduce((sum, qt) => sum + (qt.count || 0), 0); +} + +/** + * The Bloom levels this objective covers. Selection is derived, never stored + * separately: a level is selected exactly when it has a question type with a + * count above zero. Zeroing a level's last type therefore un-checks it, and a + * selected level with no way to generate anything cannot exist. + */ +export function selectedBloomLevels(questionTypes) { + const seen = []; + (questionTypes || []).forEach((qt) => { + if ((qt.count || 0) > 0 && !seen.includes(qt.bloomLevel)) seen.push(qt.bloomLevel); + }); + return seen; +} + +/** The default question type for a Bloom level, used when seeding one. */ +export function defaultTypeForLevel(bloomLevel) { + return ( + (DEFAULT_BLOOM_TYPE_PREFERENCES[bloomLevel] || [])[0] || QUESTION_TYPES.MULTIPLE_CHOICE + ); +} + +/** + * Build a breakdown for an objective saved before question types existed. + * + * Such an objective carries Bloom levels and a `questionCount` but no types, so + * under a model where selection is derived it would open with every chip grey + * and a total of zero — its configuration would look wiped. Seeding + * reconstructs an equivalent breakdown from what it does have, in memory only: + * nothing is written until the instructor saves the objective. + * + * Each level gets its default type and an even share of the count. The share is + * floored at one per level, because a level with no questions would immediately + * deselect itself and lose a level the instructor had chosen. That floor is the + * only case where the total changes — measured across 866 real legacy rows, 851 + * keep their count exactly and 15 gain a single question, all of them objectives + * with three Bloom levels but only two questions, where today the third level + * silently generates nothing anyway. + */ +export function seedQuestionTypes(bloomLevels, questionCount) { + const levels = (bloomLevels || []).filter(Boolean); + if (levels.length === 0) return []; + + const target = Math.min( + MAX_QUESTIONS_PER_OBJECTIVE, + Math.max(levels.length, parseInt(questionCount, 10) || 0) + ); + const base = Math.floor(target / levels.length); + const remainder = target % levels.length; + + return levels.map((level, index) => ({ + bloomLevel: level, + questionType: defaultTypeForLevel(level), + count: base + (index < remainder ? 1 : 0), + })); +} + +/** + * Collapse repeated (bloomLevel, questionType) pairs into one entry. + * + * Two entries naming the same pair are not two things to generate, they are one + * thing said twice — the server merges them the same way. Doing it on load + * matters because the +/- handler finds a pair by its first match: against a + * duplicated pair it would edit only one of the two entries while the displayed + * total counts both, so the stepper could never reach the number on screen. + * Objectives stored before the merge existed can still carry duplicates. + * + * A faithful re-description, so it does not clamp: the objective cap is enforced + * where counts are changed and again on save, and silently shrinking a stored + * total here would misreport what the objective is currently set to. + */ +export function mergeQuestionTypes(questionTypes) { + const merged = new Map(); + (questionTypes || []).forEach((qt) => { + if (!qt) return; + const key = JSON.stringify([qt.bloomLevel, qt.questionType]); + const existing = merged.get(key); + if (existing) existing.count += qt.count || 0; + else merged.set(key, { bloomLevel: qt.bloomLevel, questionType: qt.questionType, count: qt.count || 0 }); + }); + return [...merged.values()]; +} + +/** + * The breakdown to render for a granular objective: its own if it has one, + * otherwise a seeded equivalent. Callers never need to know which they got. + */ +export function questionTypesFor(granular) { + const existing = granular?.questionTypes; + if (Array.isArray(existing) && existing.length > 0) return mergeQuestionTypes(existing); + return seedQuestionTypes(granular?.bloomTaxonomies, granular?.questionCount); +} diff --git a/client/src/pages/QuestionGeneration.jsx b/client/src/pages/QuestionGeneration.jsx index a900dae..d9a81cc 100644 --- a/client/src/pages/QuestionGeneration.jsx +++ b/client/src/pages/QuestionGeneration.jsx @@ -258,9 +258,12 @@ export default function QuestionGeneration() { return; } group.items.forEach((item) => { - if (item.mode === "manual" && item.bloom.length === 0) { - hasBloomError = true; - } + // A Bloom level exists on an item only while it has a question type + // with a count, so "has levels" and "has something to generate" are the + // same condition — an item whose types were all zeroed arrives here with + // no levels and is caught by this one check. + if (item.mode !== "manual") return; + if (item.bloom.length === 0) hasBloomError = true; }); }); diff --git a/client/src/pages/Settings.jsx b/client/src/pages/Settings.jsx index 69ac85d..f5b55a8 100644 --- a/client/src/pages/Settings.jsx +++ b/client/src/pages/Settings.jsx @@ -19,19 +19,6 @@ import { MoodleConnectionPanel, } from "../components/lms/LmsConnectionPanels"; import { CO_INSTRUCTOR_PERMISSIONS } from "../lib/permissions"; -import { - QUESTION_TYPES, - DEFAULT_BLOOM_TYPE_PREFERENCES, - BLOOM_LEVELS, -} from "../lib/constants"; -import { BLOOM_BADGE_COLORS } from "../lib/bloom"; - -const TYPE_LABELS = { - [QUESTION_TYPES.MULTIPLE_CHOICE]: "Multiple Choice", - [QUESTION_TYPES.FILL_IN_THE_BLANK]: "Fill-in-the-blank", - [QUESTION_TYPES.CALCULATION]: "Calculation", - [QUESTION_TYPES.OPEN_ENDED]: "Open-ended", -}; // Pipeline stages a course owner can tune, keyed to the server's // OPERATION_GROUPS so the labels match what the usage report prints. @@ -182,11 +169,6 @@ export default function Settings() { const [activeTab, setActiveTab] = useState( canvasReturnState ? "canvas" : openMoodleSettings ? "moodle" : "general" ); - const [bloomPrimary, setBloomPrimary] = useState(() => - Object.fromEntries( - BLOOM_LEVELS.map((level) => [level, DEFAULT_BLOOM_TYPE_PREFERENCES[level][0]]) - ) - ); const [prompts, setPrompts] = useState(() => buildPromptState()); // Co-instructor permission toggles (owner only). Default every feature to // enabled; the stored map only carries explicit restrictions. @@ -227,16 +209,6 @@ export default function Settings() { if (settings.prompts) { setPrompts(buildPromptState(settings.prompts)); } - if (settings.bloomTypePreferences) { - setBloomPrimary((prev) => { - const next = { ...prev }; - for (const level of BLOOM_LEVELS) { - const prefs = settings.bloomTypePreferences[level]; - if (prefs && prefs.length > 0) next[level] = prefs[0]; - } - return next; - }); - } setReasoningEffort(buildEffortState(settings.reasoningEffort)); setAutoFixEnabled(settings.autoFixEnabled !== false); if (settings.coInstructorPermissions) { @@ -268,19 +240,8 @@ export default function Settings() { showToast("No course selected. Please select a course first.", "error"); return; } - // Primary first, then the default fallbacks minus the primary - const bloomTypePreferences = Object.fromEntries( - BLOOM_LEVELS.map((level) => { - const primary = bloomPrimary[level]; - const rest = DEFAULT_BLOOM_TYPE_PREFERENCES[level].filter( - (type) => type !== primary - ); - return [level, [primary, ...rest]]; - }) - ); saveMutation.mutate({ prompts, - bloomTypePreferences, // Only the owner may change co-instructor permissions or the generation // controls; the server strips them from a non-owner's update regardless. ...(isOwner @@ -297,15 +258,6 @@ export default function Settings() { }); }; - const handleResetBloom = () => { - setBloomPrimary( - Object.fromEntries( - BLOOM_LEVELS.map((level) => [level, DEFAULT_BLOOM_TYPE_PREFERENCES[level][0]]) - ) - ); - showToast("Bloom defaults restored — click Save All Changes to apply.", "info"); - }; - const handleCopyCode = async () => { if (!enrollmentCode) { showToast("No code to copy", "warning"); @@ -414,70 +366,6 @@ export default function Settings() { {activeTab === "general" && (
-
-

- Question Type by Bloom Level -

-

- Set the primary question type generated for each Bloom's Taxonomy level. - Changes apply to this course only. The default mapping is used when no - override is set. -

- -
- - - - - - - - - - {BLOOM_LEVELS.map((level) => ( - - - - - - ))} - -
Bloom's LevelPrimary Question TypeDefault
- - {level} - - - - - {TYPE_LABELS[DEFAULT_BLOOM_TYPE_PREFERENCES[level][0]]} -
-
- -
- -
-
-

Course invite code

diff --git a/client/src/pages/question-generation/AIGenerateModal.jsx b/client/src/pages/question-generation/AIGenerateModal.jsx index aba7ea7..60d10fc 100644 --- a/client/src/pages/question-generation/AIGenerateModal.jsx +++ b/client/src/pages/question-generation/AIGenerateModal.jsx @@ -111,6 +111,7 @@ export default function AIGenerateModal({ course, onClose, onSaved }) { granularObjectives: objective.granularObjectives.map((go) => ({ text: typeof go === "string" ? go : go.text, bloomTaxonomies: typeof go === "string" ? [] : go.bloomTaxonomies || [], + questionTypes: typeof go === "string" ? [] : go.questionTypes || [], })), }); if (!data.success) { diff --git a/client/src/pages/question-generation/BloomTypePanel.jsx b/client/src/pages/question-generation/BloomTypePanel.jsx new file mode 100644 index 0000000..ea4b765 --- /dev/null +++ b/client/src/pages/question-generation/BloomTypePanel.jsx @@ -0,0 +1,77 @@ +import { QUESTION_TYPES, MAX_QUESTIONS_PER_OBJECTIVE } from "../../lib/constants"; +import { pairCount } from "../../lib/questionTypes"; +import { formatQuestionTypeLabel } from "../../lib/utils"; + +const TYPE_ORDER = [ + QUESTION_TYPES.MULTIPLE_CHOICE, + QUESTION_TYPES.FILL_IN_THE_BLANK, + QUESTION_TYPES.CALCULATION, + QUESTION_TYPES.OPEN_ENDED, +]; + +// Per-Bloom-level breakdown of how many questions of each type to generate. +// Rendered below the Bloom chip row when a selected chip is expanded. +// +// Taking every count for this level to zero deselects the level: selection is +// derived from these numbers rather than tracked alongside them. +export default function BloomTypePanel({ + bloomLevel, + questionTypes, + objectiveTotal, + onChangeCount, +}) { + // pairCount sums, matching the card's total and the chip badge. Reading the + // first match instead would show a different number for the same data. + const countFor = (type) => pairCount(questionTypes, bloomLevel, type); + const objectiveFull = objectiveTotal >= MAX_QUESTIONS_PER_OBJECTIVE; + + return ( +

+
+ Question types for {bloomLevel} +
+
+ {TYPE_ORDER.map((type) => { + const count = countFor(type); + const label = formatQuestionTypeLabel(type); + return ( +
+ {label} +
+ + + {count} + + +
+
+ ); + })} +
+
+ ); +} diff --git a/client/src/pages/question-generation/ObjectiveGroupCard.jsx b/client/src/pages/question-generation/ObjectiveGroupCard.jsx index 4dbe4c9..0aa93a4 100644 --- a/client/src/pages/question-generation/ObjectiveGroupCard.jsx +++ b/client/src/pages/question-generation/ObjectiveGroupCard.jsx @@ -1,5 +1,8 @@ -import { BLOOM_LEVELS } from "../../lib/constants"; +import { useState } from "react"; +import { BLOOM_LEVELS, MAX_QUESTIONS_PER_OBJECTIVE } from "../../lib/constants"; +import { levelTotal as levelTotalOf, totalQuestions } from "../../lib/questionTypes"; import AutoGrowTextarea from "./AutoGrowTextarea"; +import BloomTypePanel from "./BloomTypePanel"; function GranularItemRow({ item, @@ -7,12 +10,27 @@ function GranularItemRow({ onToggleSelected, onCommitText, onToggleBloom, - onChangeCount, + onChangeTypeCount, onDelete, }) { + // The one Bloom level whose type panel is open, or null. Opening a level + // closes any other: several stacked panels grew the card without making the + // levels comparable, and because they were ordered by when each was clicked + // rather than by Bloom order, the stack rarely matched the chip row above it. + const [expandedBloom, setExpandedBloom] = useState(null); + const toggleBloomPanel = (level) => + setExpandedBloom((prev) => (prev === level ? null : level)); + const showBloomValidation = showValidation && item.mode === "manual" && item.bloom.length === 0; + // Every figure on this card is derived from questionTypes, through the shared + // helpers — the panel below reads the same array, and reading it two different + // ways is how the pill and the total used to disagree. + const questionTypes = item.questionTypes || []; + const total = totalQuestions(questionTypes); + const levelTotal = (level) => levelTotalOf(questionTypes, level); + return (
+ {/* A level is selected exactly when it has questions to generate, + so the chip has no separate remove control: zeroing the level's + types in its panel is what deselects it. */} {BLOOM_LEVELS.map((level) => { const isSelected = item.bloom.includes(level); + const count = levelTotal(level); + // Selecting a level adds a question, so at the objective's limit + // there is no room for a new one. Disabled rather than a click + // that quietly does nothing. + const blockedByCap = !isSelected && total >= MAX_QUESTIONS_PER_OBJECTIVE; + const disabled = item.mode === "auto" || blockedByCap; return ( ); })}
+ {/* Still guarded on item.bloom: a level deselects the moment its last + type reaches zero, and its panel must go with it. */} + {expandedBloom && item.bloom.includes(expandedBloom) && ( + qt.bloomLevel === expandedBloom)} + objectiveTotal={total} + onChangeCount={(type, delta) => onChangeTypeCount(expandedBloom, type, delta)} + /> + )} + {showBloomValidation && (
@@ -86,38 +133,20 @@ function GranularItemRow({ )}
- {/* Count stepper: how many questions to generate for this objective */} + {/* How many questions this objective will generate. Always the derived + total of its per-Bloom-level type counts — the number is adjusted in + those panels, never here, so there is nothing to disagree with. */}
Questions -
- - - {item.count} - - -
+ + {total} +
); @@ -132,7 +161,7 @@ export default function ObjectiveGroupCard({ onCommitTitle, onCommitItemText, onToggleBloom, - onChangeCount, + onChangeTypeCount, onDeleteItem, onAddGranular, onRequestDelete, @@ -287,7 +316,9 @@ export default function ObjectiveGroupCard({ } onCommitText={(value) => onCommitItemText(item, value)} onToggleBloom={(level) => onToggleBloom(item, level)} - onChangeCount={(delta) => onChangeCount(item, delta)} + onChangeTypeCount={(bloomLevel, questionType, delta) => + onChangeTypeCount(item, bloomLevel, questionType, delta) + } onDelete={() => onDeleteItem(item)} /> ))} diff --git a/client/src/pages/question-generation/ObjectivesStep.jsx b/client/src/pages/question-generation/ObjectivesStep.jsx index 9330df5..8b18081 100644 --- a/client/src/pages/question-generation/ObjectivesStep.jsx +++ b/client/src/pages/question-generation/ObjectivesStep.jsx @@ -6,6 +6,13 @@ import Modal from "../../components/ui/Modal"; import { useToast } from "../../components/ui/Toast"; import AIGenerateModal from "./AIGenerateModal"; import ObjectiveGroupCard from "./ObjectiveGroupCard"; +import { + questionTypesFor, + selectedBloomLevels, + totalQuestions, + defaultTypeForLevel, +} from "../../lib/questionTypes"; +import { MAX_QUESTIONS_PER_OBJECTIVE } from "../../lib/constants"; /* ------------------------------ Main step 1 ------------------------------ */ @@ -49,6 +56,34 @@ export default function ObjectivesStep({ ); }; + // `bloom` and `count` are projections of questionTypes, never independent + // state: a level is selected exactly when it has a type with a count, and the + // total is the sum of those counts. Every mutation goes through here so the + // three cannot drift apart — they used to, and a stale `count` was what made + // an objective generate a number of questions nobody had chosen. + const withQuestionTypes = (item, questionTypes) => ({ + ...item, + questionTypes, + bloom: selectedBloomLevels(questionTypes), + count: totalQuestions(questionTypes), + }); + + // Build the editor's view of a granular objective. Objectives saved before + // question types existed get an equivalent breakdown seeded from their Bloom + // levels and question count, so they open configured rather than blank. + const itemFromGranular = (granular, id) => + withQuestionTypes( + { + id, + granularId: granular._id ? String(granular._id) : null, + text: granular.name, + mode: "manual", + level: 1, + selected: false, + }, + questionTypesFor(granular) + ); + // Persist a group's full objective record (name, materials, granular list). // Granulars removed from this page are only detached, never deleted: the // server treats any granular missing from this payload as a deletion, so @@ -57,10 +92,12 @@ export default function ObjectivesStep({ if (!group?.objectiveId || !course?.id) return; const granularObjectives = [...group.items, ...(group.detachedItems || [])].map( (item) => { + // No questionCount: questionTypes carries the total, and a second copy + // of the same number could only ever disagree with it. const granularObj = { text: item.text, bloomTaxonomies: item.bloom || [], - questionCount: item.count, + questionTypes: item.questionTypes || [], }; if (item.granularId) granularObj.id = item.granularId; return granularObj; @@ -125,22 +162,9 @@ export default function ObjectivesStep({ title: objectiveName, isOpen: true, materialIds, - items: granularObjectives.map((granular, index) => ({ - id: parseFloat(`${newGroupNumber}.${index + 1}`), - granularId: granular._id ? String(granular._id) : null, - text: granular.name, - bloom: - granular.bloomTaxonomies && granular.bloomTaxonomies.length > 0 - ? granular.bloomTaxonomies - : [], - minQuestions: 2, - count: - granular.questionCount || - Math.max(2, granular.bloomTaxonomies?.length || 0), - mode: "manual", - level: 1, - selected: false, - })), + items: granularObjectives.map((granular, index) => + itemFromGranular(granular, parseFloat(`${newGroupNumber}.${index + 1}`)) + ), }; setObjectiveGroups((prev) => [...prev, newGroup]); } catch (error) { @@ -160,17 +184,9 @@ export default function ObjectivesStep({ title: objective.name, isOpen: true, materialIds, - items: (granulars || []).map((granular, gIdx) => ({ - id: parseFloat(`${newGroupNumber}.${gIdx + 1}`), - granularId: String(granular._id), - text: granular.name, - bloom: granular.bloomTaxonomies || [], - minQuestions: 2, - count: 2, - mode: "manual", - level: 1, - selected: false, - })), + items: (granulars || []).map((granular, gIdx) => + itemFromGranular(granular, parseFloat(`${newGroupNumber}.${gIdx + 1}`)) + ), }); }); return next; @@ -178,15 +194,21 @@ export default function ObjectivesStep({ invalidateObjectives(); }; + // Selects a Bloom level by giving it a question type to generate. A level + // with no types would immediately read as unselected, so adding the chip and + // seeding its default type are the same action. Clicking an already-selected + // chip opens its breakdown panel instead of deselecting; deselecting happens + // by zeroing the level's types there. const toggleBloomChip = (group, item, level) => { if (item.mode !== "manual") return; updateGroup(group.id, (g) => { const items = g.items.map((i) => { - if (i.id !== item.id) return i; - const bloom = i.bloom.includes(level) - ? i.bloom.filter((b) => b !== level) - : [...i.bloom, level]; - return { ...i, bloom, count: Math.max(i.count, bloom.length) }; + if (i.id !== item.id || i.bloom.includes(level)) return i; + if (totalQuestions(i.questionTypes) >= MAX_QUESTIONS_PER_OBJECTIVE) return i; + return withQuestionTypes(i, [ + ...(i.questionTypes || []), + { bloomLevel: level, questionType: defaultTypeForLevel(level), count: 1 }, + ]); }); const updated = { ...g, items }; if (g.objectiveId) saveObjectiveToDatabase(updated); @@ -194,16 +216,37 @@ export default function ObjectivesStep({ }); }; - const changeCount = (group, item, delta) => { - const minAllowed = Math.max(2, item.bloom?.length || 0); - const next = item.count + delta; - if (delta > 0 && item.count >= 9) return; - if (delta < 0 && item.count <= minAllowed) return; + // Adjust the count for one (bloomLevel, questionType) pair. Dropping a pair + // to zero removes it, and when that was the level's last type the level + // deselects — which is the only way to deselect one. Zeroing every level + // leaves the objective with no Bloom levels, which validateStep1 already + // blocks, so there is no state where an objective silently generates + // something other than what the panel shows. + const changeTypeCount = (group, item, bloomLevel, questionType, delta) => { updateGroup(group.id, (g) => { - const updated = { - ...g, - items: g.items.map((i) => (i.id === item.id ? { ...i, count: next } : i)), - }; + const items = g.items.map((i) => { + if (i.id !== item.id) return i; + const existing = i.questionTypes || []; + const idx = existing.findIndex( + (qt) => qt.bloomLevel === bloomLevel && qt.questionType === questionType + ); + if (delta > 0 && totalQuestions(existing) >= MAX_QUESTIONS_PER_OBJECTIVE) return i; + + let next; + if (idx === -1) { + if (delta <= 0) return i; + next = [...existing, { bloomLevel, questionType, count: 1 }]; + } else { + const newCount = existing[idx].count + delta; + if (newCount <= 0) { + next = existing.filter((_, j) => j !== idx); + } else { + next = existing.map((qt, j) => (j === idx ? { ...qt, count: newCount } : qt)); + } + } + return withQuestionTypes(i, next); + }); + const updated = { ...g, items }; saveObjectiveToDatabase(updated); return updated; }); @@ -254,13 +297,15 @@ export default function ObjectivesStep({ ...g, items: [ ...g.items, + // Starts with no Bloom levels, so no question types and a total of + // zero. Picking a chip seeds its type and the total follows. { id: Date.now() + g.items.length + 1, granularId: null, text: "", bloom: [], - minQuestions: 2, - count: 2, + questionTypes: [], + count: 0, mode: "manual", level: 1, selected: false, @@ -296,17 +341,25 @@ export default function ObjectivesStep({ templates(parent) .slice(0, granularCount) .forEach((template, i) => { - newItems.push({ - id: parseFloat(`${parent.id}.${i + 1}`), - text: template.title, - bloom: template.bloom, - minQuestions: 1, - count: 1, - mode: "manual", - level: 2, - parentId: parent.id, - selected: false, - }); + // Templates arrive with Bloom levels already chosen, so seed a + // type for each — otherwise they would render as unselected. + newItems.push( + withQuestionTypes( + { + id: parseFloat(`${parent.id}.${i + 1}`), + text: template.title, + mode: "manual", + level: 2, + parentId: parent.id, + selected: false, + }, + (template.bloom || []).map((level) => ({ + bloomLevel: level, + questionType: defaultTypeForLevel(level), + count: 1, + })) + ) + ); }); }); return { @@ -415,7 +468,9 @@ export default function ObjectivesStep({ onCommitTitle={(value) => commitGroupTitle(group, value)} onCommitItemText={(item, value) => commitItemText(group, item, value)} onToggleBloom={(item, level) => toggleBloomChip(group, item, level)} - onChangeCount={(item, delta) => changeCount(group, item, delta)} + onChangeTypeCount={(item, bloomLevel, questionType, delta) => + changeTypeCount(group, item, bloomLevel, questionType, delta) + } onDeleteItem={(item) => deleteItem(group, item)} onAddGranular={() => addNewGranular(group)} onRequestDelete={() => setDeleteTarget(group.id)} diff --git a/client/src/pages/question-generation/generationApi.js b/client/src/pages/question-generation/generationApi.js index 9d37a97..1070e66 100644 --- a/client/src/pages/question-generation/generationApi.js +++ b/client/src/pages/question-generation/generationApi.js @@ -79,6 +79,10 @@ export async function generateQuestions(course, objectiveGroups, onProgress, opt // Optional: pin the generated type (Question Bank wizard). Omitted for // the main pathway, where type is derived from Bloom preferences. ...(granular.questionType ? { questionType: granular.questionType } : {}), + // Optional: explicit per-Bloom-level type counts from the objective + // generation step. When present, the server generates exactly this + // breakdown instead of resolving type via course-wide preferences. + ...(granular.questionTypes?.length ? { questionTypes: granular.questionTypes } : {}), }); } catch (error) { if (error?.status === 429) { diff --git a/package.json b/package.json index 31f7306..045e2b3 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,7 @@ "type": "commonjs", "dependencies": { "@llamaindex/liteparse": "^1.5.3", - "@ubc/ubc-genai-toolkit-course-list-sync": "^0.1.3", + "@ubc/ubc-genai-toolkit-course-list-sync": "^0.1.4", "@ubc/ubc-genai-toolkit-lms-integration": "1.0.3", "archiver": "^7.0.1", "cheerio": "^1.1.2", diff --git a/src/constants/app-constants.js b/src/constants/app-constants.js index d868199..12a6985 100644 --- a/src/constants/app-constants.js +++ b/src/constants/app-constants.js @@ -70,6 +70,7 @@ INSTRUCTIONS: 8. For each granular objective, identify the most appropriate Bloom's Taxonomy level(s) based on the nature of the skill or concept being assessed (choose from: Remember, Understand, Apply, Analyze, Evaluate, Create). 9. Write each granular objective as a clear, concise statement beginning with an active verb (e.g., "Apply...", "Distinguish between...", "Derive..."). Do not add boilerplate prefixes. 10. Ensure objectives are specific to the content provided, not generic. Use the terminology from the course materials. +11. For each Bloom level you assigned to a granular objective, recommend which question type(s) would best assess it there, and how many (1-5) of each. Choose from: multiple-choice, fill-in-the-blank, calculation, open-ended. Bloom's level guidance (sample verbs in parentheses): - Remember: recall a definition or fact (define, list, identify, name) @@ -79,6 +80,13 @@ Bloom's level guidance (sample verbs in parentheses): - Evaluate: justify, critique, or defend a choice (evaluate, critique, justify, judge) - Create: design, construct, or propose something new (design, construct, propose, formulate) +Question-type guidance: +- multiple-choice: default choice when there are clear correct/incorrect alternatives — most Remember/Understand/Apply/Analyze content. +- fill-in-the-blank: short factual recall, a single term or value. +- calculation: only when the content involves a numeric formula/procedure the student must execute. +- open-ended: Create/Evaluate-level reasoning that can't be reduced to one correct option. +A Bloom level may map to more than one type when genuinely useful, but don't pad — keep totals per granular objective small (2-4) unless the material clearly supports more. + RULES FOR GRANULAR OBJECTIVES: - Each granular objective under a main objective must test a DISTINCT concept or skill — not a rephrasing of the same idea. - A granular objective must not restate the meta objective in different words; it must test a specific sub-skill the meta encompasses. @@ -96,6 +104,7 @@ SELF-CHECK BEFORE RETURNING YOUR RESPONSE: - Each granular objective is genuinely distinct and necessary — remove any that are redundant. - Every meta objective has at least one granular objective. - Every granular objective begins with an active verb and has at least one Bloom level. +- Every Bloom level on a granular objective has at least one recommended question type with a count. IMPORTANT RULES: 1. Base objectives strictly on the provided material content. @@ -127,11 +136,19 @@ Syntax & Language: - For user-provided granular objectives, preserve the instructor’s wording exactly. Only correct obvious grammatical errors. 2. Taxonomy: Every granular objective must include an array of applicable Bloom’s Taxonomy levels (Remember, Understand, Apply, Analyze, Evaluate, Create). For user-provided granular objectives, infer Bloom levels from the verb and scope of the text. 3. Alignment: All content must be derived strictly from the provided course content. Do not invent material that is not in the provided course content. +4. Question types: For each Bloom level assigned to a granular objective, recommend which question type(s) would best assess it there, and how many (1-5) of each. Choose from: multiple-choice, fill-in-the-blank, calculation, open-ended. + +Question-type guidance: +- multiple-choice: default choice when there are clear correct/incorrect alternatives — most Remember/Understand/Apply/Analyze content. +- fill-in-the-blank: short factual recall, a single term or value. +- calculation: only when the content involves a numeric formula/procedure the student must execute. +- open-ended: Create/Evaluate-level reasoning that can't be reduced to one correct option. SELF-CHECK BEFORE RETURNING YOUR RESPONSE: - No two granular objectives under the same Meta objective test the same fact or skill. - Every Meta objective has at least one granular objective. - Every granular objective has at least one Bloom level. +- Every Bloom level on a granular objective has at least one recommended question type with a count. - User-provided granular objectives still convey the instructor’s original meaning. FINAL INSTRUCTIONS: @@ -347,6 +364,26 @@ const DEFAULT_BLOOM_TYPE_PREFERENCES = { */ const MAX_MATERIALS_PER_OBJECTIVE = 3; +/** + * Ceiling on the total questions for ONE GRANULAR objective, summed across every + * Bloom level and question type it asks for. A meta objective is not capped, so a + * meta with several granulars can still ask for a multiple of this. + * + * Unlike MAX_MATERIALS_PER_OBJECTIVE this does bound cost: every question is its + * own LLM generation, and within one granular objective they run sequentially + * (they share a conversation so the prompt prefix stays cached), so an unclamped + * count is a request that never returns. + * + * Deliberately the only cap. An earlier per-pair limit constrained how the total + * was distributed as well as its size, which is a judgement about pedagogy rather + * than cost — an instructor wanting all of it as one type at one level is asking + * for no more work than spreading it around. Measured over 929 existing granular + * objectives, 97% use exactly two questions, the 99th percentile is three, and the + * most any has ever used is seven, so this leaves generous room while still ruling + * out the pathological case. + */ +const MAX_QUESTIONS_PER_OBJECTIVE = 20; + module.exports = { QUESTION_GENERATION_PROMPT, QUESTION_REVIEW_PROMPT, @@ -369,4 +406,5 @@ module.exports = { DEFAULT_PROMPTS, DEFAULT_BLOOM_TYPE_PREFERENCES, MAX_MATERIALS_PER_OBJECTIVE, + MAX_QUESTIONS_PER_OBJECTIVE, }; diff --git a/src/constants/llm-schemas.js b/src/constants/llm-schemas.js index 10db6e8..21b5f9f 100644 --- a/src/constants/llm-schemas.js +++ b/src/constants/llm-schemas.js @@ -8,12 +8,18 @@ // All object schemas set additionalProperties:false and list every property in // `required`, which is also what OpenAI strict structured outputs demands. -const { BLOOM_LEVELS } = require("./app-constants"); +const { BLOOM_LEVELS, QUESTION_TYPES } = require("./app-constants"); // Learning objectives: a relevance verdict plus { objectives: [ { name, -// granularObjectives: [ { text, bloomTaxonomies } ] } ] }. The verdict makes -// "there is no teachable content here" an explicit, schema-enforced answer -// instead of inviting the model to invent plausible-sounding objectives. +// granularObjectives: [ { text, bloomTaxonomies, questionTypes } ] } ] }. The +// verdict makes "there is no teachable content here" an explicit, +// schema-enforced answer instead of inviting the model to invent +// plausible-sounding objectives. +// +// questionTypes is a flat array of (bloomLevel, questionType, count) triples +// rather than an object keyed by Bloom level — JSON schema needs fixed +// property names for objects, and a flat array is simpler for the model to +// emit and for the caller to validate/group. const OBJECTIVES_SCHEMA = { type: "object", additionalProperties: false, @@ -38,8 +44,21 @@ const OBJECTIVES_SCHEMA = { type: "array", items: { type: "string", enum: BLOOM_LEVELS }, }, + questionTypes: { + type: "array", + items: { + type: "object", + additionalProperties: false, + properties: { + bloomLevel: { type: "string", enum: BLOOM_LEVELS }, + questionType: { type: "string", enum: Object.values(QUESTION_TYPES) }, + count: { type: "integer", minimum: 1, maximum: 5 }, + }, + required: ["bloomLevel", "questionType", "count"], + }, + }, }, - required: ["text", "bloomTaxonomies"], + required: ["text", "bloomTaxonomies", "questionTypes"], }, }, }, diff --git a/src/controllers/rag-llm.js b/src/controllers/rag-llm.js index b326221..5c31915 100644 --- a/src/controllers/rag-llm.js +++ b/src/controllers/rag-llm.js @@ -20,7 +20,7 @@ const { generationLimiter } = require('../utils/generation-limiter'); const { isRetryableLLMError } = require('../utils/llm-limiter'); const { effortForStage } = require('../utils/llm-effort'); const { OBJECTIVES_SCHEMA, QUESTION_REVIEW_SCHEMA } = require('../constants/llm-schemas'); -const { resolveGenerationQuestionType } = require('../utils/question-type-selection'); +const { resolveGenerationQuestionType, normalizeQuestionTypes } = require('../utils/question-type-selection'); const settingsService = require('../services/settings'); const questionService = require('../services/question'); const QuestionFactory = require('../models/questions/QuestionFactory'); @@ -29,7 +29,7 @@ const { getGeneratedQuestionText, normalizeQuestionText, } = require('../utils/question-generation'); -const { DEFAULT_PROMPTS, BLOOM_LEVELS, DEFAULT_BLOOM_TYPE_PREFERENCES, QUESTION_TYPES, QUESTION_REVIEW_PROMPT, QUESTION_FIX_PROMPT } = require('../constants/app-constants'); +const { DEFAULT_PROMPTS, BLOOM_LEVELS, DEFAULT_BLOOM_TYPE_PREFERENCES, QUESTION_TYPES, QUESTION_REVIEW_PROMPT, QUESTION_FIX_PROMPT, MAX_QUESTIONS_PER_OBJECTIVE } = require('../constants/app-constants'); /** * Objective-generation context size that warrants a warning. Not a limit: @@ -348,7 +348,7 @@ const searchRagHandler = async (req, res) => { const generateQuestionsWithRagHandler = async (req, res) => { try { - const { courseId, courseName, learningObjectiveId, learningObjectiveText, granularLearningObjectiveId, granularLearningObjectiveText, bloomLevels, materialIds, count, questionType: requestedQuestionType } = req.body; + const { courseId, courseName, learningObjectiveId, learningObjectiveText, granularLearningObjectiveId, granularLearningObjectiveText, bloomLevels, materialIds, count, questionType: requestedQuestionType, questionTypes } = req.body; console.log("=== RAG + LLM GENERATION REQUEST ==="); console.log("Course ID:", courseId); @@ -489,17 +489,46 @@ const generateQuestionsWithRagHandler = async (req, res) => { }); try { - // Determine question type for each bloom level using course settings - const bloomTypePrefs = settings?.bloomTypePreferences || DEFAULT_BLOOM_TYPE_PREFERENCES; - const targetCount = parseInt(count) || bloomLevels.length || 1; - // When the caller pins a type (Question Bank wizard), honour it for every - // question; otherwise fall back to the course's Bloom→type preferences. - const questionTypeForIndex = (i) => - resolveGenerationQuestionType({ - requestedType: requestedQuestionType, + // Determine the (bloomLevel, questionType) work list for this granular + // objective. When the caller supplies explicit per-Bloom-level type + // counts (the objective-generation step's typed granular items), honour + // those exactly. Otherwise fall back to the legacy behaviour: round-robin + // bloomLevels for `count` questions, resolving type via the caller's + // pinned type (Question Bank wizard) or the course's Bloom→type + // preferences. + // + // Both branches are clamped, and both matter: every entry becomes its own + // sequential LLM generation (they share a conversation to keep the prompt + // prefix cached), so an unbounded count from the request body is a request + // that never returns. The typed branch shares normalizeQuestionTypes with + // the objective-save path — the same values used to be clamped when saved + // and unbounded when generated. + let workItems = normalizeQuestionTypes(questionTypes).flatMap((qt) => + Array(qt.count).fill({ + bloomLevel: qt.bloomLevel, + questionType: qt.questionType, + }) + ); + + if (workItems.length === 0) { + const fallbackCount = Math.min( + MAX_QUESTIONS_PER_OBJECTIVE, + parseInt(count) || bloomLevels.length || 1 + ); + workItems = Array.from({ length: fallbackCount }, (_, i) => ({ bloomLevel: bloomLevels[i % bloomLevels.length] || 'Understand', - bloomTypePreferences: bloomTypePrefs, - }); + questionType: resolveGenerationQuestionType({ + requestedType: requestedQuestionType, + bloomLevel: bloomLevels[i % bloomLevels.length] || 'Understand', + }), + })); + } + + // Declared here, outside the if above, so every downstream use + // (slotSpecs, buildTurn's "QUESTION X OF Y" text, the `requested` field + // in the response) sees it regardless of which branch populated + // workItems. + const targetCount = workItems.length; // The prefix every request in this batch opens with — the planner's and // each generator's — byte-for-byte identical, so the provider processes @@ -548,10 +577,10 @@ const generateQuestionsWithRagHandler = async (req, res) => { const sharedPrefix = buildSharedPrefix(); - const slotSpecs = Array.from({ length: targetCount }, (_, i) => ({ + const slotSpecs = workItems.map((item, i) => ({ index: i, - bloomLevel: bloomLevels[i % bloomLevels.length] || "Understand", - questionType: questionTypeForIndex(i), + bloomLevel: item.bloomLevel, + questionType: item.questionType, })); let totalPromptTokens = 0; @@ -725,7 +754,7 @@ const generateQuestionsWithRagHandler = async (req, res) => { // rate limit is what stopped us, re-throw it so the handler still // answers 429 with Retry-After instead of the generic 500 below. if (pendingRateLimitError) throw pendingRateLimitError; - throw new Error(`Failed to generate any valid questions after trying all ${bloomLevels.length} bloom levels.`); + throw new Error(`Failed to generate any valid questions after trying all ${targetCount} requested question(s).`); } // Questions are always reviewed. A course owner can switch off the @@ -994,7 +1023,7 @@ Include foundational concepts, practical applications, and assessment criteria.` throw new Error("Empty response from LLM"); } - console.log("Response content:", responseContent.substring(0, 500)); + console.log("Response content:", responseContent.substring(0, 1000)); // Try to parse JSON response try { @@ -1034,7 +1063,28 @@ Include foundational concepts, practical applications, and assessment criteria.` const mappedBlooms = go.bloomTaxonomies.filter(b => validBloomLevels.includes(b)); if (mappedBlooms.length > 0) bloomTaxonomies = mappedBlooms; } - return { text, bloomTaxonomies }; + + // Keep only entries whose bloomLevel survived the filter above + // and whose type/count are valid. Any Bloom level left with no + // valid entry gets a default type so the UI never shows an + // empty per-type breakdown for a selected level. + const questionTypes = normalizeQuestionTypes(go.questionTypes, { + allowedBloomLevels: bloomTaxonomies, + }); + bloomTaxonomies.forEach((level) => { + if (!questionTypes.some((qt) => qt.bloomLevel === level)) { + // The schema asks the model for a type per Bloom level but + // cannot enforce it, so this is the only place a type is + // chosen for a level it skipped. The instructor adjusts it + // per level in the generation step, which is why there is no + // course-wide override to consult here. + const fallbackType = (DEFAULT_BLOOM_TYPE_PREFERENCES[level] || [])[0] + || QUESTION_TYPES.MULTIPLE_CHOICE; + questionTypes.push({ bloomLevel: level, questionType: fallbackType, count: 1 }); + } + }); + + return { text, bloomTaxonomies, questionTypes }; }), }; }) diff --git a/src/services/objective.js b/src/services/objective.js index 6bf7894..6b23382 100644 --- a/src/services/objective.js +++ b/src/services/objective.js @@ -2,6 +2,7 @@ const databaseService = require('./database'); const objectiveMaterialService = require('./objective-material'); const questionService = require('./question'); const { ObjectId } = require('mongodb'); +const { normalizeQuestionTypes } = require('../utils/question-type-selection'); /** * Get all parent learning objectives (parent = 0) for a specific course @@ -154,7 +155,13 @@ const createObjective = async (objectiveData) => { const granularObjectives = objectiveData.granularObjectives.map((granular) => ({ name: granular.text || granular.name, bloomTaxonomies: granular.bloomTaxonomies || [], - questionCount: granular.questionCount || 2, + // questionTypes is the sole record of how many questions an objective + // wants: the total is the sum of its counts. questionCount is no longer + // written — a second copy of the same number could only ever disagree + // with this one. Reads still tolerate it on rows that predate the change. + questionTypes: normalizeQuestionTypes(granular.questionTypes, { + allowedBloomLevels: granular.bloomTaxonomies, + }), parent: parentId, courseId: courseIdObj, createdAt: new Date(), @@ -322,14 +329,18 @@ const updateObjective = async (objectiveId, updateData) => { id: granularId, name: granular.text || granular.name, bloomTaxonomies: granular.bloomTaxonomies || [], - questionCount: granular.questionCount || 2, + questionTypes: normalizeQuestionTypes(granular.questionTypes, { + allowedBloomLevels: granular.bloomTaxonomies, + }), }); } else { // New granular objective - create it granularToCreate.push({ name: granular.text || granular.name, bloomTaxonomies: granular.bloomTaxonomies || [], - questionCount: granular.questionCount || 2, + questionTypes: normalizeQuestionTypes(granular.questionTypes, { + allowedBloomLevels: granular.bloomTaxonomies, + }), parent: id, courseId: courseIdForGranular, createdAt: new Date(), @@ -337,15 +348,19 @@ const updateObjective = async (objectiveId, updateData) => { }); } }); - + // Update existing granular objectives const updatePromises = granularToUpdate.map(granular => { const update = { name: granular.name, bloomTaxonomies: granular.bloomTaxonomies, - questionCount: granular.questionCount, + questionTypes: granular.questionTypes, updatedAt: new Date() }; + // Any stale questionCount already on the document is left in place + // rather than unset: on a row that has no questionTypes it is still the + // only record of how many questions the objective wanted, and that is + // exactly what the client reads to seed one. // Update courseId if provided if (courseIdForGranular) { update.courseId = courseIdForGranular; diff --git a/src/services/settings.js b/src/services/settings.js index 7185317..cac4fd6 100644 --- a/src/services/settings.js +++ b/src/services/settings.js @@ -1,5 +1,5 @@ const databaseService = require('./database'); -const { DEFAULT_PROMPTS, DEFAULT_BLOOM_TYPE_PREFERENCES } = require('../constants/app-constants'); +const { DEFAULT_PROMPTS } = require('../constants/app-constants'); // Mapping between hierarchical object structure and DB flat keys const KEY_MAP = { @@ -9,7 +9,6 @@ const KEY_MAP = { 'prompts.powerPointImageDescription': 'prompt_powerpoint_image_description', 'prompts.openEndedGrading': 'prompt_open_ended_grading', 'prompts.fillInTheBlankGrading': 'prompt_fill_in_the_blank_grading', - 'bloomTypePreferences': 'bloom_type_preferences', 'coInstructorPermissions': 'co_instructor_permissions', // Owner-only generation controls. The controller strips both from an update // by a non-owner, the same way it does for coInstructorPermissions. @@ -37,7 +36,6 @@ const getSettings = async (courseId) => { // Reconstruct the hierarchical settings object const settings = { prompts: {}, - bloomTypePreferences: null, coInstructorPermissions: {}, // Per-pipeline-stage reasoning effort. An absent stage falls back to // the LLM_EFFORT_* env vars and then to "medium" (see llm-effort.js), @@ -64,19 +62,6 @@ const getSettings = async (courseId) => { } } - // Resolve bloomTypePreferences: parse stored JSON or fall back to default. - const bloomDbKey = KEY_MAP['bloomTypePreferences']; - const storedBloom = settingsMap[bloomDbKey]; - if (storedBloom) { - try { - settings.bloomTypePreferences = JSON.parse(storedBloom); - } catch { - settings.bloomTypePreferences = DEFAULT_BLOOM_TYPE_PREFERENCES; - } - } else { - settings.bloomTypePreferences = DEFAULT_BLOOM_TYPE_PREFERENCES; - } - // Resolve co-instructor permissions: a map of feature key -> boolean. // An absent map (or absent key) means "allowed" — the frontend treats // anything not explicitly false as enabled, so the default is full access. @@ -130,8 +115,9 @@ const updateSettings = async (courseId, updateData) => { // Function to flatten and create bulk ops. // KEY_MAP is checked first: if the current path maps to a DB key, store it directly // (serializing objects/arrays to JSON). Only recurse into plain objects that are NOT - // themselves a top-level key — this prevents bloomTypePreferences from being - // flattened into per-level entries. + // themselves a top-level key — this prevents object-valued settings like + // coInstructorPermissions and reasoningEffort from being flattened into + // one entry per inner key. const processUpdates = (obj, prefix = '') => { for (const key in obj) { const path = prefix ? `${prefix}.${key}` : key; diff --git a/src/utils/question-type-selection.js b/src/utils/question-type-selection.js index ae22d15..9f3f3e8 100644 --- a/src/utils/question-type-selection.js +++ b/src/utils/question-type-selection.js @@ -1,4 +1,9 @@ -const { QUESTION_TYPES, DEFAULT_BLOOM_TYPE_PREFERENCES } = require('../constants/app-constants'); +const { + QUESTION_TYPES, + DEFAULT_BLOOM_TYPE_PREFERENCES, + BLOOM_LEVELS, + MAX_QUESTIONS_PER_OBJECTIVE, +} = require('../constants/app-constants'); const VALID_TYPES = new Set(Object.values(QUESTION_TYPES)); @@ -7,26 +12,78 @@ const VALID_TYPES = new Set(Object.values(QUESTION_TYPES)); * * When the caller pins a specific `requestedType` (e.g. the Question Bank * add-question wizard, where the instructor picks the type before choosing to - * generate with AI), that type wins for every question. Otherwise the type is - * derived from the course's Bloom→type preferences, falling back to - * multiple-choice. + * generate with AI), that type wins for every question. Otherwise it comes from + * the Bloom→type defaults, falling back to multiple-choice. + * + * There is no per-course override: instructors now choose types per (granular + * objective, Bloom level) in the generation step, which supersedes a course-wide + * mapping entirely. These defaults only seed that choice. * * @param {Object} params * @param {string} [params.requestedType] - Instructor-pinned question type, if any. * @param {string} [params.bloomLevel] - Bloom level for this question. - * @param {Object} [params.bloomTypePreferences] - Course Bloom→type preference map. * @returns {string} A value from QUESTION_TYPES. */ -function resolveGenerationQuestionType({ requestedType, bloomLevel, bloomTypePreferences } = {}) { +function resolveGenerationQuestionType({ requestedType, bloomLevel } = {}) { if (requestedType && VALID_TYPES.has(requestedType)) { return requestedType; } - const prefs = bloomTypePreferences || DEFAULT_BLOOM_TYPE_PREFERENCES; - const forLevel = prefs[bloomLevel]; + const forLevel = DEFAULT_BLOOM_TYPE_PREFERENCES[bloomLevel]; if (Array.isArray(forLevel) && forLevel.length > 0) { return forLevel[0]; } return QUESTION_TYPES.MULTIPLE_CHOICE; } -module.exports = { resolveGenerationQuestionType }; +/** + * Clamp and validate a granular objective's (bloomLevel, questionType, count) + * breakdown. Every caller that accepts one of these arrays from outside the + * server — the LLM's objective response, the generation request body, and the + * objective create/update payload — runs it through here, so the same values + * cannot be clamped on one path and unbounded on another. + * + * Repeated (bloomLevel, questionType) pairs are merged into one entry. Nothing + * upstream prevents them — JSON Schema cannot express "no two items share these + * two property values", so the model may emit them and a request body may carry + * them — and left alone they are ambiguous: two entries saying + * (Analyze, multiple-choice) are not two different things to generate, they are + * one thing said twice. + * + * The objective total is the only cap. How that total is divided between levels + * and types is the instructor's call: twenty questions cost the same whether they + * are one type at one level or spread across all of them. Counts are trimmed to + * whatever headroom is left rather than scaled, so earlier entries survive intact + * and the total lands exactly on the cap. + * + * @param {Array} questionTypes - Raw entries, any shape. + * @param {Object} [options] + * @param {string[]} [options.allowedBloomLevels] - Levels the entries may name. + * Defaults to every Bloom level; callers holding a specific objective pass its + * own levels so entries cannot reference a level it does not have. + * @returns {Array<{bloomLevel: string, questionType: string, count: number}>} + */ +function normalizeQuestionTypes(questionTypes, { allowedBloomLevels } = {}) { + if (!Array.isArray(questionTypes)) return []; + const allowed = Array.isArray(allowedBloomLevels) ? allowedBloomLevels : BLOOM_LEVELS; + + // Keyed on the pair itself. JSON.stringify rather than a joined string so no + // separator character can collide with a level or type name. + const merged = new Map(); + let total = 0; + for (const qt of questionTypes) { + if (!qt || !allowed.includes(qt.bloomLevel) || !VALID_TYPES.has(qt.questionType)) continue; + const room = MAX_QUESTIONS_PER_OBJECTIVE - total; + if (room <= 0) break; + + const key = JSON.stringify([qt.bloomLevel, qt.questionType]); + const existing = merged.get(key); + const count = Math.min(Math.max(1, parseInt(qt.count, 10) || 1), room); + + if (existing) existing.count += count; + else merged.set(key, { bloomLevel: qt.bloomLevel, questionType: qt.questionType, count }); + total += count; + } + return [...merged.values()]; +} + +module.exports = { resolveGenerationQuestionType, normalizeQuestionTypes }; diff --git a/tests/client/question-types.test.mjs b/tests/client/question-types.test.mjs new file mode 100644 index 0000000..022479c --- /dev/null +++ b/tests/client/question-types.test.mjs @@ -0,0 +1,202 @@ +import { describe, it, expect } from '@jest/globals'; +import { + totalQuestions, + levelTotal, + pairCount, + selectedBloomLevels, + seedQuestionTypes, + mergeQuestionTypes, + questionTypesFor, +} from '../../client/src/lib/questionTypes.js'; +import { MAX_QUESTIONS_PER_OBJECTIVE } from '../../client/src/lib/constants.js'; + +const pair = (bloomLevel, questionType, count) => ({ bloomLevel, questionType, count }); + +describe('derived totals', () => { + // The card total, the chip badge, and the panel pill all read the same array. + // They used to read it differently — reduce in two places, find in the third — + // so a duplicated pair showed three different numbers on one screen. + it('agree with each other on a duplicated pair', () => { + const types = [ + pair('Analyze', 'multiple-choice', 2), + pair('Analyze', 'multiple-choice', 3), + ]; + + expect(pairCount(types, 'Analyze', 'multiple-choice')).toBe(5); + expect(levelTotal(types, 'Analyze')).toBe(5); + expect(totalQuestions(types)).toBe(5); + }); + + it('ignore levels and types that are not being asked about', () => { + const types = [pair('Apply', 'calculation', 2), pair('Analyze', 'multiple-choice', 3)]; + + expect(pairCount(types, 'Apply', 'multiple-choice')).toBe(0); + expect(levelTotal(types, 'Evaluate')).toBe(0); + expect(totalQuestions(types)).toBe(5); + }); + + it('treat a missing array as zero rather than throwing', () => { + expect(totalQuestions(undefined)).toBe(0); + expect(levelTotal(null, 'Apply')).toBe(0); + expect(selectedBloomLevels(undefined)).toEqual([]); + }); +}); + +describe('selectedBloomLevels', () => { + // Selection is derived, not stored. This is what makes "a level the + // instructor picked that generates nothing" unrepresentable. + it('reports a level only while it has questions', () => { + expect(selectedBloomLevels([pair('Apply', 'calculation', 1)])).toEqual(['Apply']); + expect(selectedBloomLevels([pair('Apply', 'calculation', 0)])).toEqual([]); + }); + + it('lists each level once however many types it has', () => { + const types = [ + pair('Apply', 'calculation', 1), + pair('Apply', 'multiple-choice', 2), + pair('Create', 'open-ended', 1), + ]; + expect(selectedBloomLevels(types)).toEqual(['Apply', 'Create']); + }); + + it('goes empty when every type is zeroed, which is what blocks Continue', () => { + expect(selectedBloomLevels([])).toEqual([]); + }); +}); + +describe('seedQuestionTypes', () => { + // Objectives saved before question types existed have Bloom levels and a + // questionCount but no breakdown. Seeding must reconstruct an equivalent one, + // or the instructor's configuration looks wiped when they open it. + it('preserves the question count when it divides evenly', () => { + const seeded = seedQuestionTypes(['Understand', 'Apply'], 4); + + expect(totalQuestions(seeded)).toBe(4); + expect(seeded.map((e) => e.count)).toEqual([2, 2]); + }); + + it('preserves the count when it does not divide evenly', () => { + const seeded = seedQuestionTypes(['Remember', 'Understand', 'Apply'], 7); + + expect(totalQuestions(seeded)).toBe(7); + expect(seeded.map((e) => e.count)).toEqual([3, 2, 2]); + }); + + it('keeps every original Bloom level selected', () => { + const levels = ['Remember', 'Understand', 'Apply', 'Analyze', 'Evaluate', 'Create']; + expect(selectedBloomLevels(seedQuestionTypes(levels, 6))).toEqual(levels); + }); + + // The one case where the total changes. A level with zero questions would + // deselect itself, silently dropping a level the instructor had chosen, so + // the floor of one wins over preserving the count exactly. Measured across + // 866 real legacy rows this affects 15 — all "3 levels, 2 questions", where + // the round-robin generates nothing for the third level today anyway. + it('raises the total rather than dropping a level it cannot fill', () => { + const seeded = seedQuestionTypes(['Apply', 'Analyze', 'Evaluate'], 2); + + expect(totalQuestions(seeded)).toBe(3); + expect(seeded.every((e) => e.count >= 1)).toBe(true); + expect(selectedBloomLevels(seeded)).toHaveLength(3); + }); + + it('gives each level a type suited to it', () => { + const byLevel = Object.fromEntries( + seedQuestionTypes(['Remember', 'Create'], 2).map((e) => [e.bloomLevel, e.questionType]) + ); + expect(byLevel.Remember).toBe('fill-in-the-blank'); + expect(byLevel.Create).toBe('open-ended'); + }); + + // The objective total is the only cap, so a single level may hold all of it. + it('caps the objective total however few levels share it', () => { + const oneLevel = seedQuestionTypes(['Apply'], 500); + expect(oneLevel[0].count).toBe(MAX_QUESTIONS_PER_OBJECTIVE); + + const everyLevel = seedQuestionTypes( + ['Remember', 'Understand', 'Apply', 'Analyze', 'Evaluate', 'Create'], + 500 + ); + expect(totalQuestions(everyLevel)).toBe(MAX_QUESTIONS_PER_OBJECTIVE); + }); + + it('produces nothing for an objective with no Bloom levels', () => { + expect(seedQuestionTypes([], 4)).toEqual([]); + expect(seedQuestionTypes(undefined, 4)).toEqual([]); + }); + + it('still selects every level when the count is missing entirely', () => { + const seeded = seedQuestionTypes(['Understand', 'Apply'], undefined); + expect(totalQuestions(seeded)).toBe(2); + expect(selectedBloomLevels(seeded)).toEqual(['Understand', 'Apply']); + }); +}); + +describe('mergeQuestionTypes', () => { + // The +/- handler locates a pair by its first match. Against a duplicated + // pair it would edit one entry while the display counts both, so the stepper + // could never reach the number on screen. Merging on load prevents that. + it('sums a duplicated pair into one entry', () => { + const merged = mergeQuestionTypes([ + pair('Analyze', 'multiple-choice', 2), + pair('Analyze', 'multiple-choice', 3), + ]); + + expect(merged).toEqual([pair('Analyze', 'multiple-choice', 5)]); + }); + + it('leaves distinct pairs alone and keeps first-seen order', () => { + const input = [pair('Apply', 'calculation', 2), pair('Apply', 'multiple-choice', 1)]; + expect(mergeQuestionTypes(input)).toEqual(input); + }); + + // A faithful re-description: clamping here would misreport what the objective + // is currently set to. The cap is enforced where counts change and on save. + it('sums without clamping', () => { + const merged = mergeQuestionTypes([ + pair('Apply', 'calculation', 4), + pair('Apply', 'calculation', 4), + ]); + expect(merged[0].count).toBe(8); + }); + + it('leaves the totals unchanged, since a merge only re-describes them', () => { + const dup = [ + pair('Remember', 'multiple-choice', 1), + pair('Remember', 'multiple-choice', 1), + pair('Understand', 'multiple-choice', 1), + ]; + expect(totalQuestions(mergeQuestionTypes(dup))).toBe(totalQuestions(dup)); + expect(selectedBloomLevels(mergeQuestionTypes(dup))).toEqual(['Remember', 'Understand']); + }); + + it('survives a missing array', () => { + expect(mergeQuestionTypes(undefined)).toEqual([]); + expect(mergeQuestionTypes([null])).toEqual([]); + }); +}); + +describe('questionTypesFor', () => { + it('uses the objective\'s own breakdown when it has one', () => { + const own = [pair('Apply', 'calculation', 3)]; + expect(questionTypesFor({ questionTypes: own, bloomTaxonomies: ['Apply'], questionCount: 99 })) + .toEqual(own); + }); + + it('merges duplicates stored before the merge existed', () => { + const stored = { + questionTypes: [ + pair('Remember', 'multiple-choice', 1), + pair('Remember', 'multiple-choice', 1), + ], + bloomTaxonomies: ['Remember'], + }; + expect(questionTypesFor(stored)).toEqual([pair('Remember', 'multiple-choice', 2)]); + }); + + it('seeds one when the breakdown is absent or empty', () => { + const legacy = { bloomTaxonomies: ['Understand', 'Apply'], questionCount: 4 }; + expect(totalQuestions(questionTypesFor(legacy))).toBe(4); + expect(totalQuestions(questionTypesFor({ ...legacy, questionTypes: [] }))).toBe(4); + }); +}); diff --git a/tests/e2e/instructor-journey.spec.js b/tests/e2e/instructor-journey.spec.js index 6ad1d43..a79d8aa 100644 --- a/tests/e2e/instructor-journey.spec.js +++ b/tests/e2e/instructor-journey.spec.js @@ -152,26 +152,26 @@ test.describe('Instructor journey: bio_prof2 builds and publishes a quiz', () => await expect(page.getByRole('button', { name: 'Continue' })).toBeEnabled(); // Issue #31: the per-objective number must read clearly as "how many - // questions to generate". The stepper now carries a "Questions" caption, - // accessible +/- controls, and the card totals them explicitly. + // questions to generate". The card totals them explicitly. + // + // That number is always the derived total of the objective's per-Bloom-level + // question types — there is no manual stepper any more, for any objective. + // Adjusting it happens in a level's type panel, so nothing on the card can + // disagree with the breakdown underneath it. await expect(page.getByText('Questions', { exact: true }).first()).toBeVisible(); await expect( page - .getByRole('button', { - name: 'Increase questions to generate for this objective', - }) - .first() - ).toBeVisible(); - await expect( - page - .getByRole('button', { - name: 'Decrease questions to generate for this objective', - }) + .getByTitle("Total across all Bloom levels' selected question types") .first() ).toBeVisible(); await expect( page.getByText(/Total questions to generate:\s*\d+/).first() ).toBeVisible(); + // The stepper is gone rather than merely unused: an objective that reverted + // to it would generate a default breakdown nobody chose. + await expect( + page.getByRole('button', { name: /questions to generate for this objective/ }) + ).toHaveCount(0); }); test('does not invent objectives for unrelated material, but preserves instructor objectives (#32)', async () => { @@ -346,9 +346,13 @@ test.describe('Instructor journey: bio_prof2 builds and publishes a quiz', () => }); test('approves the generated questions in the question bank', async () => { - // Draft questions are selectable; select all and bulk-approve. - const rows = page.getByRole('row'); - await expect(rows.first()).toBeVisible(); + // Wait for question rows, not `rows.first()`: the first row is the header, + // which renders before the questions have loaded. Select-all is a no-op + // against an empty list, so checking it too early silently selects nothing + // and the click appears not to register. + await expect( + page.getByRole('row').filter({ hasText: /Draft|Approved/ }).first() + ).toBeVisible(); // Not .first() over all checkboxes: the page's first checkbox is the // "Show flagged only" filter, which would empty the table instead. diff --git a/tests/unit/question-type-normalization.utils.test.js b/tests/unit/question-type-normalization.utils.test.js new file mode 100644 index 0000000..4fcb031 --- /dev/null +++ b/tests/unit/question-type-normalization.utils.test.js @@ -0,0 +1,221 @@ +const { normalizeQuestionTypes } = require('../../src/utils/question-type-selection'); +const { QUESTION_TYPES, MAX_QUESTIONS_PER_OBJECTIVE } = require('../../src/constants/app-constants'); + +const pair = (bloomLevel, questionType, count) => ({ bloomLevel, questionType, count }); +const total = (entries) => entries.reduce((sum, e) => sum + e.count, 0); + +describe('normalizeQuestionTypes', () => { + describe('bounds', () => { + // Every entry becomes its own sequential LLM generation. This used to be + // clamped on the objective-save path and unbounded on the generation path, + // so the same request body meant a handful of questions when saved and + // 100,000 when generated. + it('clamps a single oversized count to the objective total', () => { + const result = normalizeQuestionTypes([pair('Understand', QUESTION_TYPES.MULTIPLE_CHOICE, 100000)]); + + expect(result).toEqual([ + pair('Understand', QUESTION_TYPES.MULTIPLE_CHOICE, MAX_QUESTIONS_PER_OBJECTIVE), + ]); + }); + + // The objective total is the only cap: how it divides between levels and + // types is the instructor's call, and one type at one level costs the same + // to generate as the same number spread around. + it('lets a single pair use the whole objective budget', () => { + const result = normalizeQuestionTypes([ + pair('Analyze', QUESTION_TYPES.MULTIPLE_CHOICE, MAX_QUESTIONS_PER_OBJECTIVE), + ]); + + expect(result).toHaveLength(1); + expect(result[0].count).toBe(MAX_QUESTIONS_PER_OBJECTIVE); + }); + + it('floors counts at one so an entry always generates something', () => { + const result = normalizeQuestionTypes([ + pair('Apply', QUESTION_TYPES.CALCULATION, 0), + pair('Analyze', QUESTION_TYPES.MULTIPLE_CHOICE, -7), + ]); + + expect(result.map((e) => e.count)).toEqual([1, 1]); + }); + + it('caps the objective total across many valid entries', () => { + const everyPair = ['Remember', 'Understand', 'Apply', 'Analyze', 'Evaluate', 'Create'] + .flatMap((level) => Object.values(QUESTION_TYPES).map((type) => pair(level, type, 5))); + + expect(total(normalizeQuestionTypes(everyPair))).toBe(MAX_QUESTIONS_PER_OBJECTIVE); + }); + + // Repetition must not be a way past the objective cap. + it('bounds a repeated pair by the objective cap, not by repetition', () => { + const repeated = Array(50).fill(pair('Analyze', QUESTION_TYPES.MULTIPLE_CHOICE, 5)); + const result = normalizeQuestionTypes(repeated); + + expect(result).toEqual([ + pair('Analyze', QUESTION_TYPES.MULTIPLE_CHOICE, MAX_QUESTIONS_PER_OBJECTIVE), + ]); + }); + + it('trims the entry that crosses the cap rather than dropping it', () => { + const result = normalizeQuestionTypes([ + pair('Apply', QUESTION_TYPES.MULTIPLE_CHOICE, 5), + pair('Analyze', QUESTION_TYPES.MULTIPLE_CHOICE, 5), + pair('Evaluate', QUESTION_TYPES.MULTIPLE_CHOICE, 5), + pair('Create', QUESTION_TYPES.MULTIPLE_CHOICE, 5), + pair('Remember', QUESTION_TYPES.FILL_IN_THE_BLANK, 5), + ]); + + expect(total(result)).toBe(MAX_QUESTIONS_PER_OBJECTIVE); + expect(result).toHaveLength(4); + expect(result[3].count).toBe(5); + }); + + it('parses string counts the way a JSON request body delivers them', () => { + const result = normalizeQuestionTypes([pair('Apply', QUESTION_TYPES.CALCULATION, '3')]); + expect(result[0].count).toBe(3); + }); + + it('falls back to one for a count that is not a number at all', () => { + const result = normalizeQuestionTypes([pair('Apply', QUESTION_TYPES.CALCULATION, 'lots')]); + expect(result[0].count).toBe(1); + }); + }); + + describe('merging repeated pairs', () => { + // Two entries naming the same pair are one thing said twice, not two things + // to generate. JSON Schema cannot express "no two items share these two + // property values", so the model can emit them — the e2e stub does exactly + // this — and a request body can carry them. + it('sums a duplicated pair into one entry', () => { + const result = normalizeQuestionTypes([ + pair('Analyze', QUESTION_TYPES.MULTIPLE_CHOICE, 2), + pair('Analyze', QUESTION_TYPES.MULTIPLE_CHOICE, 3), + ]); + + expect(result).toEqual([pair('Analyze', QUESTION_TYPES.MULTIPLE_CHOICE, 5)]); + }); + + it('preserves the total when merging (the same questions, described once)', () => { + // The shape the e2e stub produces: a repeated pair plus a distinct one. + const result = normalizeQuestionTypes([ + pair('Remember', QUESTION_TYPES.MULTIPLE_CHOICE, 1), + pair('Remember', QUESTION_TYPES.MULTIPLE_CHOICE, 1), + pair('Understand', QUESTION_TYPES.MULTIPLE_CHOICE, 1), + ]); + + expect(result).toEqual([ + pair('Remember', QUESTION_TYPES.MULTIPLE_CHOICE, 2), + pair('Understand', QUESTION_TYPES.MULTIPLE_CHOICE, 1), + ]); + expect(total(result)).toBe(3); + }); + + it('keeps the same level distinct across different types', () => { + const result = normalizeQuestionTypes([ + pair('Apply', QUESTION_TYPES.MULTIPLE_CHOICE, 1), + pair('Apply', QUESTION_TYPES.CALCULATION, 2), + ]); + + expect(result).toHaveLength(2); + expect(total(result)).toBe(3); + }); + + it('keeps the same type distinct across different levels', () => { + const result = normalizeQuestionTypes([ + pair('Apply', QUESTION_TYPES.MULTIPLE_CHOICE, 1), + pair('Analyze', QUESTION_TYPES.MULTIPLE_CHOICE, 2), + ]); + + expect(result).toHaveLength(2); + expect(total(result)).toBe(3); + }); + + it('sums duplicates without inventing a per-pair ceiling', () => { + const result = normalizeQuestionTypes([ + pair('Apply', QUESTION_TYPES.CALCULATION, 4), + pair('Apply', QUESTION_TYPES.CALCULATION, 4), + ]); + + expect(result).toEqual([pair('Apply', QUESTION_TYPES.CALCULATION, 8)]); + }); + + it('keeps first-seen order', () => { + const result = normalizeQuestionTypes([ + pair('Understand', QUESTION_TYPES.OPEN_ENDED, 1), + pair('Apply', QUESTION_TYPES.CALCULATION, 1), + pair('Understand', QUESTION_TYPES.OPEN_ENDED, 1), + ]); + + expect(result.map((e) => e.bloomLevel)).toEqual(['Understand', 'Apply']); + expect(result[0].count).toBe(2); + }); + + // Merging must not consume budget a later distinct pair still needs. + it('merges a repeat and still admits a later distinct pair', () => { + const result = normalizeQuestionTypes([ + pair('Apply', QUESTION_TYPES.MULTIPLE_CHOICE, 5), + pair('Apply', QUESTION_TYPES.MULTIPLE_CHOICE, 5), + pair('Analyze', QUESTION_TYPES.OPEN_ENDED, 2), + ]); + + expect(result).toEqual([ + pair('Apply', QUESTION_TYPES.MULTIPLE_CHOICE, 10), + pair('Analyze', QUESTION_TYPES.OPEN_ENDED, 2), + ]); + }); + }); + + describe('filtering', () => { + it('drops entries naming an unknown question type', () => { + const result = normalizeQuestionTypes([ + pair('Apply', 'true-false', 3), + pair('Apply', QUESTION_TYPES.CALCULATION, 2), + ]); + + expect(result).toEqual([pair('Apply', QUESTION_TYPES.CALCULATION, 2)]); + }); + + it('drops entries naming an unknown Bloom level', () => { + const result = normalizeQuestionTypes([pair('Synthesize', QUESTION_TYPES.MULTIPLE_CHOICE, 2)]); + expect(result).toEqual([]); + }); + + // Callers holding a specific objective pass its own levels, so the model + // cannot attach question types to a level the objective does not have. + it('restricts entries to the levels the caller allows', () => { + const result = normalizeQuestionTypes( + [ + pair('Apply', QUESTION_TYPES.MULTIPLE_CHOICE, 2), + pair('Evaluate', QUESTION_TYPES.OPEN_ENDED, 2), + ], + { allowedBloomLevels: ['Apply'] } + ); + + expect(result).toEqual([pair('Apply', QUESTION_TYPES.MULTIPLE_CHOICE, 2)]); + }); + + it('survives malformed input without throwing', () => { + expect(normalizeQuestionTypes(undefined)).toEqual([]); + expect(normalizeQuestionTypes(null)).toEqual([]); + expect(normalizeQuestionTypes('not an array')).toEqual([]); + expect(normalizeQuestionTypes([null, undefined, {}, 42])).toEqual([]); + }); + + it('keeps only the three fields it owns', () => { + const result = normalizeQuestionTypes([ + { bloomLevel: 'Apply', questionType: QUESTION_TYPES.CALCULATION, count: 2, injected: true }, + ]); + + expect(Object.keys(result[0]).sort()).toEqual(['bloomLevel', 'count', 'questionType']); + }); + }); + + it('leaves a well-formed breakdown untouched', () => { + const good = [ + pair('Understand', QUESTION_TYPES.MULTIPLE_CHOICE, 2), + pair('Apply', QUESTION_TYPES.CALCULATION, 1), + ]; + + expect(normalizeQuestionTypes(good)).toEqual(good); + }); +}); diff --git a/tests/unit/question-type-selection.utils.test.js b/tests/unit/question-type-selection.utils.test.js index 724c4b6..97d4484 100644 --- a/tests/unit/question-type-selection.utils.test.js +++ b/tests/unit/question-type-selection.utils.test.js @@ -6,65 +6,56 @@ describe('resolveGenerationQuestionType', () => { it.each(Object.values(QUESTION_TYPES))( 'returns the requested type "%s" regardless of Bloom level', (requestedType) => { - const result = resolveGenerationQuestionType({ - requestedType, - bloomLevel: 'Create', - bloomTypePreferences: DEFAULT_BLOOM_TYPE_PREFERENCES, - }); + const result = resolveGenerationQuestionType({ requestedType, bloomLevel: 'Create' }); expect(result).toBe(requestedType); } ); - it('honours the requested type even when it differs from the Bloom preference', () => { - // Create prefers open-ended first; requesting calculation must win. + it('honours the requested type even when it differs from the Bloom default', () => { + // Create defaults to open-ended first; requesting calculation must win. const result = resolveGenerationQuestionType({ requestedType: QUESTION_TYPES.CALCULATION, bloomLevel: 'Create', - bloomTypePreferences: DEFAULT_BLOOM_TYPE_PREFERENCES, }); expect(result).toBe(QUESTION_TYPES.CALCULATION); }); }); - describe('when no type is requested (main generation pathway)', () => { - it('falls back to the first preferred type for the Bloom level', () => { - const result = resolveGenerationQuestionType({ - bloomLevel: 'Remember', - bloomTypePreferences: DEFAULT_BLOOM_TYPE_PREFERENCES, - }); - expect(result).toBe(DEFAULT_BLOOM_TYPE_PREFERENCES.Remember[0]); + describe('when no type is requested', () => { + // There is no per-course override any more: instructors choose types per + // (granular objective, Bloom level) in the generation step, which supersedes + // a course-wide mapping. These defaults only seed that choice. + it('falls back to the default type for the Bloom level', () => { + expect(resolveGenerationQuestionType({ bloomLevel: 'Remember' })) + .toBe(DEFAULT_BLOOM_TYPE_PREFERENCES.Remember[0]); + expect(resolveGenerationQuestionType({ bloomLevel: 'Understand' })) + .toBe(DEFAULT_BLOOM_TYPE_PREFERENCES.Understand[0]); }); - it('uses default preferences when none are supplied', () => { - const result = resolveGenerationQuestionType({ bloomLevel: 'Understand' }); - expect(result).toBe(DEFAULT_BLOOM_TYPE_PREFERENCES.Understand[0]); + it('defaults to multiple-choice for an unknown Bloom level', () => { + const result = resolveGenerationQuestionType({ bloomLevel: 'NotABloomLevel' }); + expect(result).toBe(QUESTION_TYPES.MULTIPLE_CHOICE); }); - it('defaults to multiple-choice for an unknown Bloom level', () => { + it('ignores a course preference map if one is somehow still passed', () => { + // Guards the removal: a stale caller passing the old argument must not + // resurrect course-wide overrides through the back door. const result = resolveGenerationQuestionType({ - bloomLevel: 'NotABloomLevel', - bloomTypePreferences: DEFAULT_BLOOM_TYPE_PREFERENCES, + bloomLevel: 'Remember', + bloomTypePreferences: { Remember: [QUESTION_TYPES.OPEN_ENDED] }, }); - expect(result).toBe(QUESTION_TYPES.MULTIPLE_CHOICE); + expect(result).toBe(DEFAULT_BLOOM_TYPE_PREFERENCES.Remember[0]); }); }); describe('input hardening', () => { - it('ignores an invalid requested type and falls back to preferences', () => { - const result = resolveGenerationQuestionType({ - requestedType: 'essay', - bloomLevel: 'Apply', - bloomTypePreferences: DEFAULT_BLOOM_TYPE_PREFERENCES, - }); + it('ignores an invalid requested type and falls back to the default', () => { + const result = resolveGenerationQuestionType({ requestedType: 'essay', bloomLevel: 'Apply' }); expect(result).toBe(DEFAULT_BLOOM_TYPE_PREFERENCES.Apply[0]); }); it('ignores an empty requested type', () => { - const result = resolveGenerationQuestionType({ - requestedType: '', - bloomLevel: 'Analyze', - bloomTypePreferences: DEFAULT_BLOOM_TYPE_PREFERENCES, - }); + const result = resolveGenerationQuestionType({ requestedType: '', bloomLevel: 'Analyze' }); expect(result).toBe(DEFAULT_BLOOM_TYPE_PREFERENCES.Analyze[0]); }); @@ -72,12 +63,9 @@ describe('resolveGenerationQuestionType', () => { expect(resolveGenerationQuestionType()).toBe(QUESTION_TYPES.MULTIPLE_CHOICE); }); - it('defaults to multiple-choice when a Bloom level maps to an empty list', () => { - const result = resolveGenerationQuestionType({ - bloomLevel: 'Custom', - bloomTypePreferences: { Custom: [] }, - }); - expect(result).toBe(QUESTION_TYPES.MULTIPLE_CHOICE); + it('defaults to multiple-choice when the Bloom level is missing', () => { + expect(resolveGenerationQuestionType({ requestedType: undefined })) + .toBe(QUESTION_TYPES.MULTIPLE_CHOICE); }); }); }); diff --git a/tests/unit/settings.service.test.js b/tests/unit/settings.service.test.js index eaa13c6..3045829 100644 --- a/tests/unit/settings.service.test.js +++ b/tests/unit/settings.service.test.js @@ -3,10 +3,7 @@ jest.mock('../../src/services/database', () => ({ })); const databaseService = require('../../src/services/database'); -const { - DEFAULT_BLOOM_TYPE_PREFERENCES, - DEFAULT_PROMPTS, -} = require('../../src/constants/app-constants'); +const { DEFAULT_PROMPTS } = require('../../src/constants/app-constants'); const settingsService = require('../../src/services/settings'); function mockSettingsCollection(rows = []) { @@ -33,10 +30,6 @@ describe('settings service', () => { name: 'prompt_powerpoint_image_description', value: 'Custom PowerPoint prompt', }, - { - name: 'bloom_type_preferences', - value: JSON.stringify({ Remember: ['multiple-choice'] }), - }, { name: 'co_instructor_permissions', value: JSON.stringify({ settings: false, createQuiz: true }), @@ -50,24 +43,31 @@ describe('settings service', () => { objectiveGenerationManual: DEFAULT_PROMPTS.objectiveGenerationManual, powerPointImageDescription: 'Custom PowerPoint prompt', }, - bloomTypePreferences: { Remember: ['multiple-choice'] }, coInstructorPermissions: { settings: false, createQuiz: true }, }); }); it('falls back to defaults when stored JSON is missing or malformed', async () => { - mockSettingsCollection([ - { name: 'bloom_type_preferences', value: '{not json' }, - { name: 'co_instructor_permissions', value: '{also bad' }, - ]); + mockSettingsCollection([{ name: 'co_instructor_permissions', value: '{also bad' }]); await expect(settingsService.getSettings('course-1')).resolves.toMatchObject({ prompts: DEFAULT_PROMPTS, - bloomTypePreferences: DEFAULT_BLOOM_TYPE_PREFERENCES, coInstructorPermissions: {}, }); }); + // Courses configured before the per-course Bloom→type mapping was retired + // still have the row. It must not reappear on the settings object, or a + // caller could start honouring a preference the UI no longer exposes. + it('ignores a leftover bloom_type_preferences row', async () => { + mockSettingsCollection([ + { name: 'bloom_type_preferences', value: JSON.stringify({ Remember: ['open-ended'] }) }, + ]); + + const settings = await settingsService.getSettings('course-1'); + expect(settings.bloomTypePreferences).toBeUndefined(); + }); + it('logs and rethrows database read errors', async () => { jest.spyOn(console, 'error').mockImplementation(() => {}); databaseService.connect.mockRejectedValue(new Error('connect failed')); @@ -95,6 +95,8 @@ describe('settings service', () => { objectiveGenerationAuto: 'Updated objective prompt', powerPointImageDescription: 'Updated PowerPoint prompt', }, + // Retired: must be dropped like any other unsupported key rather than + // written back to a row nothing reads. bloomTypePreferences: { Create: ['open-ended'] }, coInstructorPermissions: { settings: false }, ignored: { nested: 'value' }, @@ -151,20 +153,6 @@ describe('settings service', () => { upsert: true, }, }, - { - updateOne: { - filter: { name: 'bloom_type_preferences', courseId: 'course-1' }, - update: { - $set: { - name: 'bloom_type_preferences', - value: JSON.stringify({ Create: ['open-ended'] }), - courseId: 'course-1', - updatedAt: expect.any(Date), - }, - }, - upsert: true, - }, - }, { updateOne: { filter: { name: 'co_instructor_permissions', courseId: 'course-1' },