fix(question-types): realign registry ids with the question_type table
The registry hardcoded numeric ids that drifted from the database `question_type` table: MATCHING and HOTSPOT were inserted mid-list (ids 9 and 11) but never seeded, shifting ORDERING/LINK/WORD_BANK/ STAR/VIDEO_RECORDING one or two ids past their real rows. Every id emitter inherited the wrong values — the exercise editor, the AI question generator, and the grading/scoring gate — so e.g. an ORDERING question was stored as id 10 (LINK in the DB), came back as a manual type, and never auto-graded. - Move MATCHING/HOTSPOT to the end (ids 14/15) and flag them `disabled`, restoring the append-only id convention and realigning ORDERING=9, LINK=10, WORD_BANK=11, STAR=12, VIDEO_RECORDING=13 with the DB. - Add a registry test asserting every enabled type's id matches the canonical `question_type` rows and disabled types stay past the live range, so ids can't silently drift again. - Constrain the agent tool schema to enabled, DB-backed ids so the model can't emit a disabled id with no row. - Drop disabled types from the teacher prompt's type list and guidance. - Centralize the dashboard question-type picker on the `disabled` flag. This also fixes video-recording detection, which compared against the stale id 15 instead of the real 13.
This commit is contained in:
@@ -23,18 +23,13 @@ const QUESTION_TYPE_LABEL_KEYS: Partial<Record<QuestionTypeKey, string>> = {
|
||||
'course.navItem.lessons.exercises.all_exercises.edit_mode.question_types.video_recording'
|
||||
};
|
||||
|
||||
// Temp hidden: MATCHING, HOTSPOT
|
||||
const HIDDEN_QUESTION_TYPE_KEYS = new Set<QuestionTypeKey>([QUESTION_TYPE_KEY.MATCHING, QUESTION_TYPE_KEY.HOTSPOT]);
|
||||
|
||||
export const QUESTION_TYPES = QUESTION_TYPE_REGISTRY.filter((qt) => !HIDDEN_QUESTION_TYPE_KEYS.has(qt.key)).map(
|
||||
(questionType) => ({
|
||||
export const QUESTION_TYPES = QUESTION_TYPE_REGISTRY.filter((qt) => !qt.disabled).map((questionType) => ({
|
||||
id: questionType.id,
|
||||
key: questionType.key,
|
||||
label: questionType.label,
|
||||
labelKey: QUESTION_TYPE_LABEL_KEYS[questionType.key],
|
||||
autoGradable: questionType.autoGradable
|
||||
})
|
||||
);
|
||||
}));
|
||||
|
||||
export const DEFAULT_QUESTION_TYPE = QUESTION_TYPES.find(
|
||||
(questionType) => questionType.key === QUESTION_TYPE_KEY.RADIO
|
||||
|
||||
@@ -13,14 +13,18 @@ export type BuildTeacherSystemPromptOptions = {
|
||||
};
|
||||
|
||||
function buildQuestionTypeListBlock(isOrgOnPaidPlan: boolean): string {
|
||||
const allowed = QUESTION_TYPE_REGISTRY.filter((t) => isOrgOnPaidPlan || !PREMIUM_QUESTION_TYPE_KEYS.has(t.key));
|
||||
// `disabled` types are built but have no `question_type` DB row, so they must
|
||||
// never be advertised to the model (the tool schema rejects their ids).
|
||||
const offered = QUESTION_TYPE_REGISTRY.filter((t) => !t.disabled);
|
||||
const allowed = offered.filter((t) => isOrgOnPaidPlan || !PREMIUM_QUESTION_TYPE_KEYS.has(t.key));
|
||||
const listing = allowed.map((t) => `- ${t.id} = ${t.typename} — ${t.label}`).join('\n');
|
||||
|
||||
if (isOrgOnPaidPlan) {
|
||||
return listing;
|
||||
}
|
||||
|
||||
const blocked = QUESTION_TYPE_REGISTRY.filter((t) => PREMIUM_QUESTION_TYPE_KEYS.has(t.key))
|
||||
const blocked = offered
|
||||
.filter((t) => PREMIUM_QUESTION_TYPE_KEYS.has(t.key))
|
||||
.map((t) => t.typename)
|
||||
.join(', ');
|
||||
|
||||
@@ -59,15 +63,13 @@ Every question you pass to \`create_exercise\` or \`add_questions\` MUST include
|
||||
- **SHORT_ANSWER** — recalling a specific term, command, value, or short phrase. Use when there's a small set of right answers you can list.
|
||||
- **NUMERIC** — anything quantitative: math, calculations, counts, sizes, rates. Default for any "how many / what value / calculate X" question.
|
||||
- **FILL_BLANK** — testing syntax, sequence completion, or code/sentence patterns where position matters ("\`SELECT \_\_ FROM users\`").
|
||||
- **MATCHING** — pairing related concepts (term ↔ definition, problem ↔ solution, tool ↔ purpose). Use whenever you'd otherwise write multiple RADIO questions all asking "what is the definition of X?".
|
||||
- **ORDERING** — sequencing steps, ranking by criterion, timeline reconstruction. Use for any "what's the right order" question instead of RADIO with shuffled options.
|
||||
- **WORD_BANK** — vocab application, classification, filling multiple blanks from a shared pool. Stronger than RADIO when the same concept set applies to many slots.
|
||||
- **TEXTAREA** — open-ended explanation, reflection, written analysis. Use sparingly (manual grading).
|
||||
- **HOTSPOT** — identifying a region of an image (UI element, anatomy, map). Use when there's a visual the lesson already shows.
|
||||
- **CHECKBOX (multi-select)** — when the question genuinely has multiple correct answers a learner should identify together. Don't use CHECKBOX as a "harder RADIO" — only when "select all that apply" is the natural verb.
|
||||
- **RADIO (single answer)** — concept-level discrimination between mutually exclusive options. Useful but the most overused — when in doubt, ask if one of the types above fits better.
|
||||
|
||||
**Anti-pattern to avoid:** an exercise of 8 questions that's 7 RADIO + 1 TRUE_FALSE. That's a "different types" technicality, not real variety. The mix must reflect the *content*: numeric questions get NUMERIC, ordering questions get ORDERING, definition-matching gets MATCHING, etc.
|
||||
**Anti-pattern to avoid:** an exercise of 8 questions that's 7 RADIO + 1 TRUE_FALSE. That's a "different types" technicality, not real variety. The mix must reflect the *content*: numeric questions get NUMERIC, ordering questions get ORDERING, fill-in patterns get FILL_BLANK, etc.
|
||||
|
||||
## Plan Mode vs Agent Mode
|
||||
|
||||
@@ -339,7 +341,7 @@ When you create an exercise (especially during plan implementation), it must act
|
||||
- For NUMERIC / STAR / WORD_BANK: still ensure the answer is unambiguously derivable from the lesson.
|
||||
|
||||
### Per-exercise structure
|
||||
- Vary question types in the same exercise — minimum 2 distinct \`questionTypeId\` values for any 3–5 question exercise, minimum 4 for 6+ questions, with no single type exceeding half the exercise. Pick the type that matches the cognitive skill (see "Question Types" above): numeric questions get NUMERIC, ordering questions get ORDERING, definition-matching gets MATCHING — do not paper over a missing fit by defaulting to RADIO.
|
||||
- Vary question types in the same exercise — minimum 2 distinct \`questionTypeId\` values for any 3–5 question exercise, minimum 4 for 6+ questions, with no single type exceeding half the exercise. Pick the type that matches the cognitive skill (see "Question Types" above): numeric questions get NUMERIC, ordering questions get ORDERING, fill-in patterns get FILL_BLANK — do not paper over a missing fit by defaulting to RADIO.
|
||||
- Set non-zero \`points\` per question (default 1; harder questions can be 2).
|
||||
|
||||
### Comprehensive final examination (when implementing a full course plan)
|
||||
|
||||
@@ -5,11 +5,22 @@ import {
|
||||
LANDING_PAGE_METADATA_DESCRIPTION_SECTION_HINT,
|
||||
LANDING_PAGE_SECTION_HTML_AGENT_HINT
|
||||
} from '@cio/ai-assistant/tools';
|
||||
import { QUESTION_TYPE_REGISTRY } from '@cio/question-types';
|
||||
import { ENABLED_QUESTION_TYPE_REGISTRY } from '@cio/question-types';
|
||||
import { ZExerciseSectionAfterBehavior } from '@cio/utils/validation/exercise';
|
||||
import { ZCourseLandingPageUpdate, ZCourseLandingPageMetadataUpdateFields } from '@cio/utils/validation/course';
|
||||
|
||||
// courseId is NOT a parameter — it's injected from the authenticated request context.
|
||||
|
||||
// Only question types backed by a `question_type` DB row may be generated.
|
||||
// Disabled types (e.g. MATCHING/HOTSPOT) have no row and would fail the FK, so
|
||||
// the model is constrained to the enabled ids rather than the whole registry.
|
||||
const ENABLED_QUESTION_TYPE_IDS = ENABLED_QUESTION_TYPE_REGISTRY.map((type) => type.id);
|
||||
const zEnabledQuestionTypeId = z
|
||||
.number()
|
||||
.int()
|
||||
.refine((id) => ENABLED_QUESTION_TYPE_IDS.includes(id), {
|
||||
message: `questionTypeId must be one of: ${ENABLED_QUESTION_TYPE_IDS.join(', ')}`
|
||||
});
|
||||
// This prevents prompt injection from tricking the LLM into targeting another course.
|
||||
|
||||
export const emptyParam = z.object({});
|
||||
@@ -63,12 +74,7 @@ export const updateContentParam = z.object({
|
||||
|
||||
export const questionSchema = z.object({
|
||||
question: z.string().min(1),
|
||||
questionTypeId: z
|
||||
.number()
|
||||
.int()
|
||||
.min(1)
|
||||
.max(QUESTION_TYPE_REGISTRY.length)
|
||||
.describe(
|
||||
questionTypeId: zEnabledQuestionTypeId.describe(
|
||||
'Required. Use the numeric question type IDs from the teacher system prompt (Question Types). Omitting this field is invalid — set an explicit type on every question and vary types within each exercise.'
|
||||
),
|
||||
points: z.number().min(0).default(1),
|
||||
@@ -193,7 +199,7 @@ export const updateQuestionPatchSchema = z
|
||||
.object({
|
||||
id: z.number().int(),
|
||||
question: z.string().min(1).optional(),
|
||||
questionTypeId: z.number().int().min(1).max(QUESTION_TYPE_REGISTRY.length).optional(),
|
||||
questionTypeId: zEnabledQuestionTypeId.optional(),
|
||||
points: z.number().min(0).optional(),
|
||||
order: z.number().int().min(0).optional(),
|
||||
exerciseSectionId: z
|
||||
|
||||
@@ -74,29 +74,11 @@ export const QUESTION_TYPE_REGISTRY: readonly QuestionTypeMetadata[] = [
|
||||
supportsPartialCredit: false,
|
||||
manualGradingRequired: true
|
||||
},
|
||||
{
|
||||
key: QUESTION_TYPE_KEY.MATCHING,
|
||||
typename: 'MATCHING',
|
||||
label: 'Matching',
|
||||
id: 9,
|
||||
autoGradable: true,
|
||||
supportsPartialCredit: true,
|
||||
manualGradingRequired: false
|
||||
},
|
||||
{
|
||||
key: QUESTION_TYPE_KEY.ORDERING,
|
||||
typename: 'ORDERING',
|
||||
label: 'Ordering',
|
||||
id: 10,
|
||||
autoGradable: true,
|
||||
supportsPartialCredit: true,
|
||||
manualGradingRequired: false
|
||||
},
|
||||
{
|
||||
key: QUESTION_TYPE_KEY.HOTSPOT,
|
||||
typename: 'HOTSPOT',
|
||||
label: 'Hotspot',
|
||||
id: 11,
|
||||
id: 9,
|
||||
autoGradable: true,
|
||||
supportsPartialCredit: true,
|
||||
manualGradingRequired: false
|
||||
@@ -105,7 +87,7 @@ export const QUESTION_TYPE_REGISTRY: readonly QuestionTypeMetadata[] = [
|
||||
key: QUESTION_TYPE_KEY.LINK,
|
||||
typename: 'LINK',
|
||||
label: 'Links',
|
||||
id: 12,
|
||||
id: 10,
|
||||
autoGradable: false,
|
||||
supportsPartialCredit: false,
|
||||
manualGradingRequired: true
|
||||
@@ -114,7 +96,7 @@ export const QUESTION_TYPE_REGISTRY: readonly QuestionTypeMetadata[] = [
|
||||
key: QUESTION_TYPE_KEY.WORD_BANK,
|
||||
typename: 'WORD_BANK',
|
||||
label: 'Word bank',
|
||||
id: 13,
|
||||
id: 11,
|
||||
autoGradable: true,
|
||||
supportsPartialCredit: true,
|
||||
manualGradingRequired: false
|
||||
@@ -123,7 +105,7 @@ export const QUESTION_TYPE_REGISTRY: readonly QuestionTypeMetadata[] = [
|
||||
key: QUESTION_TYPE_KEY.STAR,
|
||||
typename: 'STAR',
|
||||
label: 'Star rating',
|
||||
id: 14,
|
||||
id: 12,
|
||||
autoGradable: true,
|
||||
supportsPartialCredit: false,
|
||||
manualGradingRequired: false
|
||||
@@ -132,13 +114,40 @@ export const QUESTION_TYPE_REGISTRY: readonly QuestionTypeMetadata[] = [
|
||||
key: QUESTION_TYPE_KEY.VIDEO_RECORDING,
|
||||
typename: 'VIDEO_RECORDING',
|
||||
label: 'Video recording',
|
||||
id: 15,
|
||||
id: 13,
|
||||
autoGradable: false,
|
||||
supportsPartialCredit: false,
|
||||
manualGradingRequired: true
|
||||
},
|
||||
// Built but not offered in the product. Kept at the END of the id sequence
|
||||
// (append-only) so they never shift the ids of types that exist in the DB.
|
||||
// They have no `question_type` row, so `disabled` keeps them out of pickers,
|
||||
// the AI tool enum, and validation.
|
||||
{
|
||||
key: QUESTION_TYPE_KEY.MATCHING,
|
||||
typename: 'MATCHING',
|
||||
label: 'Matching',
|
||||
id: 14,
|
||||
autoGradable: true,
|
||||
supportsPartialCredit: true,
|
||||
manualGradingRequired: false,
|
||||
disabled: true
|
||||
},
|
||||
{
|
||||
key: QUESTION_TYPE_KEY.HOTSPOT,
|
||||
typename: 'HOTSPOT',
|
||||
label: 'Hotspot',
|
||||
id: 15,
|
||||
autoGradable: true,
|
||||
supportsPartialCredit: true,
|
||||
manualGradingRequired: false,
|
||||
disabled: true
|
||||
}
|
||||
] as const;
|
||||
|
||||
/** Types actually offered in the product (excludes `disabled` ones with no DB row). */
|
||||
export const ENABLED_QUESTION_TYPE_REGISTRY = QUESTION_TYPE_REGISTRY.filter((type) => !type.disabled);
|
||||
|
||||
export const QUESTION_TYPE_BY_KEY = Object.freeze(
|
||||
QUESTION_TYPE_REGISTRY.reduce(
|
||||
(acc, type) => {
|
||||
|
||||
@@ -8,4 +8,6 @@ export interface QuestionTypeMetadata {
|
||||
autoGradable: boolean;
|
||||
supportsPartialCredit: boolean;
|
||||
manualGradingRequired: boolean;
|
||||
/** Built in code but not offered in the product (no `question_type` row). */
|
||||
disabled?: boolean;
|
||||
}
|
||||
|
||||
@@ -82,26 +82,10 @@ const EXPECTED_REGISTRY = [
|
||||
supportsPartialCredit: false,
|
||||
manualGradingRequired: true
|
||||
},
|
||||
{
|
||||
key: QUESTION_TYPE_KEY.MATCHING,
|
||||
typename: 'MATCHING',
|
||||
id: 9,
|
||||
autoGradable: true,
|
||||
supportsPartialCredit: true,
|
||||
manualGradingRequired: false
|
||||
},
|
||||
{
|
||||
key: QUESTION_TYPE_KEY.ORDERING,
|
||||
typename: 'ORDERING',
|
||||
id: 10,
|
||||
autoGradable: true,
|
||||
supportsPartialCredit: true,
|
||||
manualGradingRequired: false
|
||||
},
|
||||
{
|
||||
key: QUESTION_TYPE_KEY.HOTSPOT,
|
||||
typename: 'HOTSPOT',
|
||||
id: 11,
|
||||
id: 9,
|
||||
autoGradable: true,
|
||||
supportsPartialCredit: true,
|
||||
manualGradingRequired: false
|
||||
@@ -109,7 +93,7 @@ const EXPECTED_REGISTRY = [
|
||||
{
|
||||
key: QUESTION_TYPE_KEY.LINK,
|
||||
typename: 'LINK',
|
||||
id: 12,
|
||||
id: 10,
|
||||
autoGradable: false,
|
||||
supportsPartialCredit: false,
|
||||
manualGradingRequired: true
|
||||
@@ -117,7 +101,7 @@ const EXPECTED_REGISTRY = [
|
||||
{
|
||||
key: QUESTION_TYPE_KEY.WORD_BANK,
|
||||
typename: 'WORD_BANK',
|
||||
id: 13,
|
||||
id: 11,
|
||||
autoGradable: true,
|
||||
supportsPartialCredit: true,
|
||||
manualGradingRequired: false
|
||||
@@ -125,7 +109,7 @@ const EXPECTED_REGISTRY = [
|
||||
{
|
||||
key: QUESTION_TYPE_KEY.STAR,
|
||||
typename: 'STAR',
|
||||
id: 14,
|
||||
id: 12,
|
||||
autoGradable: true,
|
||||
supportsPartialCredit: false,
|
||||
manualGradingRequired: false
|
||||
@@ -133,13 +117,51 @@ const EXPECTED_REGISTRY = [
|
||||
{
|
||||
key: QUESTION_TYPE_KEY.VIDEO_RECORDING,
|
||||
typename: 'VIDEO_RECORDING',
|
||||
id: 15,
|
||||
id: 13,
|
||||
autoGradable: false,
|
||||
supportsPartialCredit: false,
|
||||
manualGradingRequired: true
|
||||
},
|
||||
{
|
||||
key: QUESTION_TYPE_KEY.MATCHING,
|
||||
typename: 'MATCHING',
|
||||
id: 14,
|
||||
autoGradable: true,
|
||||
supportsPartialCredit: true,
|
||||
manualGradingRequired: false,
|
||||
disabled: true
|
||||
},
|
||||
{
|
||||
key: QUESTION_TYPE_KEY.HOTSPOT,
|
||||
typename: 'HOTSPOT',
|
||||
id: 15,
|
||||
autoGradable: true,
|
||||
supportsPartialCredit: true,
|
||||
manualGradingRequired: false,
|
||||
disabled: true
|
||||
}
|
||||
] as const;
|
||||
|
||||
// Canonical `question_type` rows in the database (the source of truth ids are
|
||||
// resolved against). Ids 1-13 are the live rows in insertion order; MATCHING and
|
||||
// HOTSPOT are intentionally absent (built-but-disabled). This list guards against
|
||||
// the registry id ever drifting from the DB again.
|
||||
const CANONICAL_DB_QUESTION_TYPES: ReadonlyArray<{ id: number; typename: string }> = [
|
||||
{ id: 1, typename: 'RADIO' },
|
||||
{ id: 2, typename: 'CHECKBOX' },
|
||||
{ id: 3, typename: 'TEXTAREA' },
|
||||
{ id: 4, typename: 'TRUE_FALSE' },
|
||||
{ id: 5, typename: 'SHORT_ANSWER' },
|
||||
{ id: 6, typename: 'NUMERIC' },
|
||||
{ id: 7, typename: 'FILL_BLANK' },
|
||||
{ id: 8, typename: 'FILE_UPLOAD' },
|
||||
{ id: 9, typename: 'ORDERING' },
|
||||
{ id: 10, typename: 'LINK' },
|
||||
{ id: 11, typename: 'WORD_BANK' },
|
||||
{ id: 12, typename: 'STAR' },
|
||||
{ id: 13, typename: 'VIDEO_RECORDING' }
|
||||
];
|
||||
|
||||
describe('question type registry', () => {
|
||||
it('contains exactly the supported question type keys', () => {
|
||||
expect(new Set(QUESTION_TYPE_REGISTRY.map((entry) => entry.key))).toEqual(
|
||||
@@ -176,6 +198,29 @@ describe('question type registry', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('registry ids match the canonical question_type DB rows (no drift)', () => {
|
||||
for (const row of CANONICAL_DB_QUESTION_TYPES) {
|
||||
const key = QUESTION_TYPE_TYPENAME_TO_KEY[row.typename];
|
||||
expect(key, `typename ${row.typename} missing from registry`).toBeDefined();
|
||||
expect(QUESTION_TYPE_IDS[key], `id mismatch for ${row.typename}`).toBe(row.id);
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps disabled types appended after every DB-backed type', () => {
|
||||
const dbIds = new Set(CANONICAL_DB_QUESTION_TYPES.map((row) => row.id));
|
||||
const maxDbId = Math.max(...CANONICAL_DB_QUESTION_TYPES.map((row) => row.id));
|
||||
|
||||
for (const entry of QUESTION_TYPE_REGISTRY) {
|
||||
if (entry.disabled) {
|
||||
// Disabled types have no DB row and must sit past the live id range.
|
||||
expect(dbIds.has(entry.id), `disabled ${entry.typename} must not reuse a DB id`).toBe(false);
|
||||
expect(entry.id).toBeGreaterThan(maxDbId);
|
||||
} else {
|
||||
expect(dbIds.has(entry.id), `enabled ${entry.typename} must map to a DB row`).toBe(true);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it('returns undefined for unknown typename and id lookups', () => {
|
||||
expect(getQuestionTypeByTypename('UNKNOWN')).toBeUndefined();
|
||||
expect(getQuestionTypeById(999)).toBeUndefined();
|
||||
|
||||
Reference in New Issue
Block a user