Merge pull request #29 from renzorlive/feat/content-validator

feat: two-tier content validator — schema + repository-wide semantics

Author: renzorlive

CHANGELOG.md

@@ -5,9 +5,15 @@ Format: [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Versioning: se
 
 ## [Unreleased]
 
+### Added
+- **Two-tier content validator** (issue #26): per-lesson schema rules now also check `initialCursor` within buffer bounds and validate every `solution` key token; a new repository-wide semantic pass catches what per-file validation can't — duplicate lesson IDs (S001), dangling `prerequisites` references (S002), duplicate curriculum order (S003), and index/content drift in both directions (S004). Every violation is reported with `file path → schema location`, all in one run. 14 new tests cover every violation class.
+
 ### Changed
 - **Tailwind is now built, not fetched** (TD-10): the ~300 KB render-blocking `cdn.tailwindcss.com` script (explicitly not for production) is replaced by a 32 KB static `css/tailwind.css` generated at build time from the classes actually used (`npm run build:css`, Tailwind v3 — same major as the CDN, guaranteeing visual parity). Wired into `npm run check`/CI; the last third-party console warning is gone, and offline/PWA work is unblocked.
 
+### Fixed
+- **Practice lessons are validated again**: the contract runner iterated the `vimLessons` Map with `Object.entries()` (always empty), silently skipping all 17 practice lessons — the suite now checks all 35 lessons instead of 18. Same Map-migration bug family as the dead Practice buttons fixed in v3.0.0.
+
 ## [3.0.0] - 2026-07-10 — Community Alpha
 
 > First tagged release. Everything below shipped together as the Community Alpha; nothing earlier was ever tagged.

js/levels.js

@@ -18,11 +18,12 @@ export const levels = loadRegularLessons();
 // specified in docs/architecture/level-lifecycle.md (ADR-0005); parallel
 // initialization code paths are not accepted.
 //
-// TODO(validateLessonSchema, issue #26): when the JSON content system lands
-// (docs/ContentSystem.md, Phase 2), replace these runtime guards with a
-// schema validator run in CI over every content file: cursor within buffer,
-// buffer shape, objective validity, solution replay.
-// https://github.com/renzorlive/vimmaster/issues/26
+// Schema validation is enforced in CI by the content validator
+// (tests/contract/: per-lesson rules + semantic-rules.js — issue #26):
+// cursor within buffer, buffer shape, objective validity, solution key
+// tokens, unique IDs, resolvable prerequisites, index consistency; the
+// Golden Suite replays every solution. The guards below remain as
+// runtime defense-in-depth for content loaded outside CI.
 
 // Exactly one of these must be present on every lesson — it is the objective.
 const WIN_CONDITION_PROPS = ['target', 'targetText', 'targetContent', 'exCommands'];

tests/contract/rules/cursor.js

@@ -5,20 +5,47 @@ registerRule({
     name: 'Cursor Invalid',
     severity: 'error',
     validate: (lesson, report) => {
-        // We currently do not have a declarative `startCursor` property.
-        // The cursor is set via the `setup(gameState)` function in code.
-        // We can check if `setup` is a function, and maybe stringify it to ensure it sets cursor.
-        // In the future (V2), we will have a declarative `startCursor` field.
+        // Declarative start position: must be a well-formed {row, col} that
+        // sits INSIDE the initial buffer (issue #26, Tier 1). The runtime
+        // clamp in initializeLessonState remains as defense-in-depth; CI is
+        // the enforcement point.
+        if (lesson.initialCursor !== undefined) {
+            const cursor = lesson.initialCursor;
+            if (typeof cursor !== 'object' || cursor === null) {
+                report('`initialCursor` must be an object {row, col}.', 'initialCursor');
+            } else if (!Number.isInteger(cursor.row) || !Number.isInteger(cursor.col)) {
+                report('`initialCursor` must contain integer `row` and `col`.', 'initialCursor');
+            } else if (Array.isArray(lesson.initialContent) && lesson.initialContent.length > 0) {
+                const { row, col } = cursor;
+                if (row < 0 || row >= lesson.initialContent.length) {
+                    report(
+                        `\`initialCursor.row\` ${row} is outside the buffer (0..${lesson.initialContent.length - 1}).`,
+                        'initialCursor.row'
+                    );
+                } else {
+                    const line = lesson.initialContent[row];
+                    const maxCol = Math.max(0, (typeof line === 'string' ? line.length : 1) - 1);
+                    if (col < 0 || col > maxCol) {
+                        report(
+                            `\`initialCursor.col\` ${col} is outside line ${row} (0..${maxCol}).`,
+                            'initialCursor.col'
+                        );
+                    }
+                }
+            }
+        }
+
+        // Legacy fields kept for backward compatibility
         if (lesson.startCursor !== undefined) {
             if (typeof lesson.startCursor !== 'object' || lesson.startCursor === null) {
                 report('`startCursor` must be an object {row, col}.', 'startCursor');
             } else if (typeof lesson.startCursor.row !== 'number' || typeof lesson.startCursor.col !== 'number') {
                 report('`startCursor` must contain numeric `row` and `col`.', 'startCursor');
             }
         }
-        
-        // For legacy `setup`, we can't fully validate the exact bounds statically without executing it,
-        // but if it is provided, it must be a function.
+
+        // For legacy `setup`, we can't fully validate the exact bounds statically
+        // without executing it, but if it is provided, it must be a function.
         if (lesson.setup !== undefined && typeof lesson.setup !== 'function') {
             report('Legacy `setup` must be a function.', 'setup');
         }

tests/contract/rules/solution.js

@@ -15,6 +15,26 @@ registerRule({
             );
         } else if (lesson.solution.length === 0) {
             report('The `solution` array is empty.', 'solution');
+        } else {
+            // Each entry must be a valid key token: a single printable
+            // character, a named key, or a Ctrl+<char> chord — the formats
+            // the Golden replay understands (issue #26: invalid command
+            // sequences).
+            const NAMED_KEYS = new Set(['Enter', 'Escape', 'Backspace', '<Esc>']);
+            lesson.solution.forEach((key, i) => {
+                const valid =
+                    typeof key === 'string' &&
+                    (key.length === 1 ||
+                        NAMED_KEYS.has(key) ||
+                        /^Ctrl\+.$/i.test(key));
+                if (!valid) {
+                    report(
+                        `\`solution[${i}]\` is not a valid key token: ${JSON.stringify(key)}.`,
+                        `solution[${i}]`,
+                        "Use single characters ('d'), named keys ('Enter', 'Escape', 'Backspace'), or chords ('Ctrl+r')."
+                    );
+                }
+            });
         }
     }
 });

tests/contract/runner.js

@@ -1,6 +1,7 @@
 import { validateLesson } from './validator.js';
+import { validateContentSet, lessonFilePath } from './semantic-rules.js';
 
-// Import all rules to register them
+// Import all per-lesson rules to register them (Tier 1)
 import './rules/id.js';
 import './rules/cursor.js';
 import './rules/buffer.js';
@@ -12,38 +13,50 @@ import './rules/metadata.js';
 // Import content to validate
 import { levels } from '../../js/levels.js';
 import { vimLessons } from '../../js/cheat-mode.js';
+import { loadIndex } from '../../js/content-loader.js';
 
-// Aggregate all lessons
+// Aggregate all lessons — regular levels AND practice lessons.
+// (vimLessons is a Map: it must be iterated with .entries(); the previous
+// Object.entries() returned nothing, silently skipping all practice lessons.)
 const allLessons = [];
 
-// From levels.js (indexed)
 levels.forEach((lesson, index) => {
     allLessons.push({ lesson, fallbackId: `Level ${index}` });
 });
 
-// From cheat-mode.js (object map)
-for (const [key, lesson] of Object.entries(vimLessons)) {
-    allLessons.push({ lesson, fallbackId: `CheatMode ${key}` });
+for (const [key, lesson] of vimLessons.entries()) {
+    allLessons.push({ lesson, fallbackId: `Practice ${key}` });
 }
 
 let totalErrors = 0;
 let totalWarnings = 0;
 let totalInfos = 0;
 
+// Reporting contract (issue #26): every violation is printed with the
+// lesson's file path and schema location; ALL violations across ALL lessons
+// are collected in one run; a single non-zero exit happens at the end.
+const printViolation = (severityColor, ruleId, filePath, location, message, suggestion) => {
+    console.log(`  ${severityColor}${ruleId}\x1b[0m ${filePath} → ${location || '(lesson)'}`);
+    console.log(`    ${message}`);
+    if (suggestion) {
+        console.log(`    ↳ \x1b[90m${suggestion}\x1b[0m`);
+    }
+};
+
 console.log('\n🔍 Running VIM Master Contract Suite\n');
 
+// ---- Tier 1: per-lesson (schema-level) ------------------------------------
 for (const { lesson, fallbackId } of allLessons) {
     const errors = validateLesson(lesson, fallbackId);
-    
+
     if (errors.length === 0) {
         console.log(`✓ ${lesson.name || fallbackId}`);
         continue;
     }
 
-    // Determine the highest severity for this lesson to color the status
     const hasError = errors.some(e => e.severity === 'error');
     const hasWarning = errors.some(e => e.severity === 'warning');
-    
+
     if (hasError) {
         console.log(`\x1b[31m✗\x1b[0m ${lesson.name || fallbackId}`);
     } else if (hasWarning) {
@@ -52,25 +65,38 @@ for (const { lesson, fallbackId } of allLessons) {
         console.log(`\x1b[36mℹ\x1b[0m ${lesson.name || fallbackId}`);
     }
 
-    // Print all violations
     for (const err of errors) {
+        const filePath = lessonFilePath(lesson);
         if (err.severity === 'error') {
             totalErrors++;
-            console.log(`  \x1b[31m${err.ruleId || 'ERROR'}\x1b[0m ${err.message}`);
+            printViolation('\x1b[31m', err.ruleId || 'ERROR', filePath, err.field, err.message, err.suggestion);
         } else if (err.severity === 'warning') {
             totalWarnings++;
-            console.log(`  \x1b[33m${err.ruleId || 'WARN'}\x1b[0m ${err.message}`);
+            printViolation('\x1b[33m', err.ruleId || 'WARN', filePath, err.field, err.message, err.suggestion);
         } else {
             totalInfos++;
-            console.log(`  \x1b[36m${err.ruleId || 'INFO'}\x1b[0m ${err.message}`);
-        }
-        
-        if (err.suggestion) {
-            console.log(`    ↳ \x1b[90m${err.suggestion}\x1b[0m`);
+            printViolation('\x1b[36m', err.ruleId || 'INFO', filePath, err.field, err.message, err.suggestion);
         }
     }
 }
 
+// ---- Tier 2: repository-wide (semantic) ------------------------------------
+console.log('\n🧭 Semantic checks (whole content set)\n');
+
+const semanticViolations = validateContentSet(
+    allLessons.map(({ lesson }) => lesson),
+    loadIndex()
+);
+
+if (semanticViolations.length === 0) {
+    console.log('✓ IDs unique, prerequisites resolve, curriculum order consistent, index in sync');
+} else {
+    for (const v of semanticViolations) {
+        totalErrors++;
+        printViolation('\x1b[31m', v.ruleId, v.filePath, v.location, v.message, v.suggestion);
+    }
+}
+
 console.log('\n--------------------------------');
 console.log(`Lessons checked: ${allLessons.length}`);
 console.log(`\x1b[31mErrors:   ${totalErrors}\x1b[0m`);

tests/contract/semantic-rules.js

@@ -0,0 +1,102 @@
+/**
+ * Repository-wide (semantic) content validation — Tier 2 of the content
+ * validator (issue #26).
+ *
+ * Per-lesson rules (tests/contract/rules/*) can only see one lesson at a
+ * time; these rules receive the FULL content set and catch what per-file
+ * validation cannot: duplicate IDs, dangling cross-lesson references, and
+ * curriculum-ordering inconsistencies.
+ *
+ * Reporting contract (issue #26): every violation carries the lesson's
+ * file path and a schema location, all violations are collected in one
+ * run, and the caller exits non-zero once at the end.
+ */
+
+export const lessonFilePath = (lesson) =>
+    lesson && typeof lesson.id === 'string' ? `content/lessons/${lesson.id}.json` : '<unknown lesson file>';
+
+/**
+ * @param {Array<Object>} lessons - every lesson in the content set
+ * @param {Object} index - parsed content/index.json ({ regularLessons, practiceLessons })
+ * @returns {Array<{ruleId, severity, filePath, location, message, suggestion}>}
+ */
+export function validateContentSet(lessons, index) {
+    const violations = [];
+    const report = (ruleId, lesson, location, message, suggestion = null) => {
+        violations.push({
+            ruleId,
+            severity: 'error',
+            filePath: lessonFilePath(lesson),
+            location,
+            message,
+            suggestion
+        });
+    };
+
+    // S001 — lesson IDs must be unique across the whole content set
+    const byId = new Map();
+    for (const lesson of lessons) {
+        if (typeof lesson.id !== 'string') continue; // per-lesson L001 reports this
+        if (byId.has(lesson.id)) {
+            report('S001', lesson, 'id',
+                `Duplicate lesson ID '${lesson.id}' (also defined by another lesson).`,
+                'Lesson IDs must be unique across content/lessons/.');
+        } else {
+            byId.set(lesson.id, lesson);
+        }
+    }
+
+    // S002 — cross-lesson references must resolve
+    for (const lesson of lessons) {
+        const prerequisites = lesson.metadata?.prerequisites;
+        if (!Array.isArray(prerequisites)) continue; // per-lesson L012 reports this
+        for (const ref of prerequisites) {
+            if (!byId.has(ref)) {
+                report('S002', lesson, 'metadata.prerequisites',
+                    `Prerequisite '${ref}' does not match any lesson ID.`,
+                    'Reference an existing lesson id from content/lessons/.');
+            }
+        }
+    }
+
+    // S003 — curriculum order must be unique within each track
+    const orderSeen = new Map(); // order number -> first lesson
+    for (const lesson of lessons) {
+        const order = lesson.metadata?.order;
+        if (typeof order !== 'number') continue; // per-lesson L012 reports this
+        if (orderSeen.has(order)) {
+            report('S003', lesson, 'metadata.order',
+                `Curriculum order ${order} is already used by '${orderSeen.get(order).id}'.`,
+                'Each lesson needs a distinct metadata.order.');
+        } else {
+            orderSeen.set(order, lesson);
+        }
+    }
+
+    // S004 — content/index.json must agree with the lesson set, both ways
+    if (index && Array.isArray(index.regularLessons) && Array.isArray(index.practiceLessons)) {
+        const indexed = [...index.regularLessons, ...index.practiceLessons];
+        for (const id of indexed) {
+            if (!byId.has(id)) {
+                violations.push({
+                    ruleId: 'S004',
+                    severity: 'error',
+                    filePath: 'content/index.json',
+                    location: 'regularLessons/practiceLessons',
+                    message: `Index lists '${id}' but no such lesson exists.`,
+                    suggestion: 'Regenerate the index with `npm run build:content`.'
+                });
+            }
+        }
+        const indexedSet = new Set(indexed);
+        for (const lesson of lessons) {
+            if (typeof lesson.id === 'string' && !indexedSet.has(lesson.id)) {
+                report('S004', lesson, 'id',
+                    `Lesson '${lesson.id}' exists but is missing from content/index.json.`,
+                    'Regenerate the index with `npm run build:content`.');
+            }
+        }
+    }
+
+    return violations;
+}