fix: stop warning about merged themes in curriculum schedule
When the KB has more than 26 themes the curriculum AI is required to merge — so theme labels not appearing as week names are expected, not warnings. The previous validation surfaced this as a console.warn that read like an error in the learning station console. - validateSchedule now only flags missing theme labels when themes_kb <= 26 - adds a real coverage check: warns when learning topics are absent from every week (the actual signal we care about) - adds vitest coverage for both behaviours Closes #12 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -60,8 +60,16 @@ export function buildThemeTopicMap(topics) {
|
||||
|
||||
/**
|
||||
* Validates a 26-week schedule against the provided topics.
|
||||
* Checks for exactly 26 weeks, duration range, theme existence, and topic existence.
|
||||
* Returns { valid: boolean, errors: string[] }
|
||||
*
|
||||
* Warning policy:
|
||||
* - Theme names not appearing as a week label are NOT a warning when the KB
|
||||
* has more than 26 themes — the AI is required to merge in that case, and
|
||||
* topic_ids from the merged themes are carried through under the chosen
|
||||
* week label. We only warn about missing themes when merging wasn't needed.
|
||||
* - Real coverage is measured on TOPICS: a topic that exists in the KB but
|
||||
* is absent from every week's topic_ids is a genuine gap and gets a warning.
|
||||
*
|
||||
* Returns { valid: boolean, errors: string[], warnings: string[] }
|
||||
*/
|
||||
export function validateSchedule(schedule, topics) {
|
||||
const errors = []; // Hard errors — schedule is unusable
|
||||
@@ -71,10 +79,13 @@ export function validateSchedule(schedule, topics) {
|
||||
errors.push(`Schedule must contain exactly 26 weeks. Found ${schedule?.length || 0}.`);
|
||||
}
|
||||
|
||||
const validThemes = new Set(topics.filter(t => t.type !== 'fact' && t.learning_relevance !== 'exclude').map(t => t.theme || 'General'));
|
||||
const learningTopics = topics.filter(t => t.type !== 'fact' && t.learning_relevance !== 'exclude');
|
||||
const validThemes = new Set(learningTopics.map(t => t.theme || 'General'));
|
||||
const validTopicIds = new Set(topics.map(t => t.id));
|
||||
const learningTopicIds = new Set(learningTopics.map(t => t.id));
|
||||
|
||||
const scheduledThemes = new Set();
|
||||
const scheduledTopicIds = new Set();
|
||||
|
||||
for (let i = 0; i < (schedule || []).length; i++) {
|
||||
const week = schedule[i];
|
||||
@@ -86,7 +97,7 @@ export function validateSchedule(schedule, topics) {
|
||||
}
|
||||
// Allow AI-merged theme names — only flag truly unknown themes as warnings
|
||||
scheduledThemes.add(week.theme);
|
||||
|
||||
|
||||
if (!week.topic_ids || week.topic_ids.length === 0) {
|
||||
errors.push(`Week ${week.week_number} has no topic_ids.`);
|
||||
} else {
|
||||
@@ -94,20 +105,30 @@ export function validateSchedule(schedule, topics) {
|
||||
if (!validTopicIds.has(tId)) {
|
||||
errors.push(`Week ${week.week_number} references unknown topic_id: ${tId}`);
|
||||
}
|
||||
scheduledTopicIds.add(tId);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Theme coverage — soft warnings, not hard errors
|
||||
// When there are more themes than 26 weeks, the AI must merge some.
|
||||
const missingThemes = [];
|
||||
for (const t of validThemes) {
|
||||
if (!scheduledThemes.has(t)) {
|
||||
missingThemes.push(t);
|
||||
// Theme coverage — only warn when merging wasn't required. When themes_kb > 26
|
||||
// the AI is *required* to merge, so absent theme labels are expected.
|
||||
if (validThemes.size <= 26) {
|
||||
const missingThemes = [];
|
||||
for (const t of validThemes) {
|
||||
if (!scheduledThemes.has(t)) {
|
||||
missingThemes.push(t);
|
||||
}
|
||||
}
|
||||
if (missingThemes.length > 0) {
|
||||
warnings.push(`${missingThemes.length} theme(s) not scheduled: ${missingThemes.join(', ')}`);
|
||||
}
|
||||
}
|
||||
if (missingThemes.length > 0) {
|
||||
warnings.push(`${missingThemes.length} theme(s) not directly scheduled (topics may appear under merged themes): ${missingThemes.join(', ')}`);
|
||||
|
||||
// Topic coverage — the real signal. Topics carried under a merged theme are
|
||||
// still covered; topics absent from every week are not.
|
||||
const missingTopicCount = [...learningTopicIds].filter(id => !scheduledTopicIds.has(id)).length;
|
||||
if (missingTopicCount > 0) {
|
||||
warnings.push(`${missingTopicCount} learning topic(s) not covered by any week of the schedule.`);
|
||||
}
|
||||
|
||||
return { valid: errors.length === 0, errors, warnings };
|
||||
@@ -224,7 +245,9 @@ Rules:
|
||||
throw new Error(`Generated schedule failed validation after retry:\n- ${validationResult.errors.join('\n- ')}`);
|
||||
}
|
||||
|
||||
// Log warnings but don't fail
|
||||
// Log warnings but don't fail. With themes_kb > 26 the AI must merge themes,
|
||||
// so most "missing theme" noise is filtered upstream in validateSchedule —
|
||||
// anything that lands here is a genuine coverage gap worth surfacing.
|
||||
if (validationResult.warnings.length > 0) {
|
||||
console.warn('[Curriculum] Schedule generated with warnings:', validationResult.warnings);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user