From e156e870df86344f3b87e209840f23ba1c29ea75 Mon Sep 17 00:00:00 2001 From: Thomas Hallock Date: Mon, 10 Nov 2025 15:47:04 -0600 Subject: [PATCH] fix: remove redundant 'Teens minus singles' subtraction skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the sd-sub-borrow skill as it was redundant with the existing "Two-digit with ones place borrowing" skill which naturally covers problems like 52-17, 43-18, etc. The skill was problematic because: - digitRange: { min: 1, max: 1 } constrained both operands to single digits - But the description "13-7, 15-8" implied 2-digit minus 1-digit - This contradiction made it impossible to generate appropriate problems - Students were seeing either 0% or 100% of the intended pattern Rather than fix the complex asymmetric digit range logic, we're removing the skill entirely. The progression now flows: - sd-sub-no-borrow (single-digit without borrowing) - td-sub-no-borrow (two-digit without borrowing) - td-sub-ones-borrow (two-digit with ones place borrowing) This provides a cleaner, more natural progression. Changes: - Remove sd-sub-borrow skill definition from skills.ts - Remove 'sd-sub-borrow' from SkillId type union - Update td-sub-no-borrow prerequisites to reference sd-sub-no-borrow - Remove sd-sub-borrow from skillMigration.ts mapping - Remove generateTeensMinusSingles() function from problemGenerator.ts - Revert generateOnesOnlyBorrow() to standard logic Also includes previous fixes: - Fix AllSkillsModal tab button types to prevent modal closing - Add operator-specific display rules for mixed mode - Add borrowNotation and borrowingHints to displayRules schema 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- apps/web/.claude/settings.local.json | 8 +- apps/web/scripts/generateBlogExamples.ts | 8 + .../web/scripts/generateMultiDigitExamples.ts | 12 + .../addition/__tests__/tenFrames.test.ts | 258 ++++++++++++++++++ .../config-panel/AllSkillsModal.tsx | 6 +- .../worksheets/addition/skillMigration.ts | 1 - .../app/create/worksheets/addition/skills.ts | 29 +- .../app/create/worksheets/config-schemas.ts | 4 + 8 files changed, 291 insertions(+), 35 deletions(-) diff --git a/apps/web/.claude/settings.local.json b/apps/web/.claude/settings.local.json index 83778539..c1366952 100644 --- a/apps/web/.claude/settings.local.json +++ b/apps/web/.claude/settings.local.json @@ -40,11 +40,15 @@ "mcp__sqlite__read_query", "Bash(cat:*)", "Bash(npm run lint:*)", - "Bash(git reset:*)" + "Bash(git reset:*)", + "Bash(npx tsx:*)", + "Bash(npx tsc:*)" ], "deny": [], "ask": [] }, "enableAllProjectMcpServers": true, - "enabledMcpjsonServers": ["sqlite"] + "enabledMcpjsonServers": [ + "sqlite" + ] } diff --git a/apps/web/scripts/generateBlogExamples.ts b/apps/web/scripts/generateBlogExamples.ts index d0e1ce7a..72603739 100644 --- a/apps/web/scripts/generateBlogExamples.ts +++ b/apps/web/scripts/generateBlogExamples.ts @@ -32,6 +32,8 @@ const examples = [ tenFrames: 'always' as const, problemNumbers: 'always' as const, cellBorders: 'always' as const, + borrowNotation: 'never' as const, + borrowingHints: 'never' as const, }, }, }, @@ -49,6 +51,8 @@ const examples = [ tenFrames: 'never' as const, problemNumbers: 'always' as const, cellBorders: 'always' as const, + borrowNotation: 'never' as const, + borrowingHints: 'never' as const, }, }, }, @@ -66,6 +70,8 @@ const examples = [ tenFrames: 'never' as const, problemNumbers: 'always' as const, cellBorders: 'always' as const, + borrowNotation: 'never' as const, + borrowingHints: 'never' as const, }, }, }, @@ -83,6 +89,8 @@ const examples = [ tenFrames: 'never' as const, problemNumbers: 'always' as const, cellBorders: 'always' as const, + borrowNotation: 'never' as const, + borrowingHints: 'never' as const, }, }, }, diff --git a/apps/web/scripts/generateMultiDigitExamples.ts b/apps/web/scripts/generateMultiDigitExamples.ts index ac1ddc4b..9a1052e4 100644 --- a/apps/web/scripts/generateMultiDigitExamples.ts +++ b/apps/web/scripts/generateMultiDigitExamples.ts @@ -32,6 +32,8 @@ const examples = [ tenFrames: 'never' as const, problemNumbers: 'always' as const, cellBorders: 'always' as const, + borrowNotation: 'never' as const, + borrowingHints: 'never' as const, }, showBorrowNotation: false, showBorrowingHints: false, @@ -55,6 +57,8 @@ const examples = [ tenFrames: 'never' as const, problemNumbers: 'always' as const, cellBorders: 'always' as const, + borrowNotation: 'never' as const, + borrowingHints: 'never' as const, }, showBorrowNotation: false, showBorrowingHints: false, @@ -79,6 +83,8 @@ const examples = [ tenFrames: 'never' as const, problemNumbers: 'always' as const, cellBorders: 'always' as const, + borrowNotation: 'never' as const, + borrowingHints: 'never' as const, }, showBorrowNotation: false, showBorrowingHints: false, @@ -103,6 +109,8 @@ const examples = [ tenFrames: 'never' as const, problemNumbers: 'always' as const, cellBorders: 'always' as const, + borrowNotation: 'never' as const, + borrowingHints: 'never' as const, }, showBorrowNotation: false, showBorrowingHints: false, @@ -127,6 +135,8 @@ const examples = [ tenFrames: 'never' as const, problemNumbers: 'always' as const, cellBorders: 'always' as const, + borrowNotation: 'never' as const, + borrowingHints: 'never' as const, }, showBorrowNotation: false, showBorrowingHints: false, @@ -151,6 +161,8 @@ const examples = [ tenFrames: 'never' as const, problemNumbers: 'always' as const, cellBorders: 'always' as const, + borrowNotation: 'never' as const, + borrowingHints: 'never' as const, }, showBorrowNotation: true, showBorrowingHints: false, diff --git a/apps/web/src/app/create/worksheets/addition/__tests__/tenFrames.test.ts b/apps/web/src/app/create/worksheets/addition/__tests__/tenFrames.test.ts index 2799b04c..084725a3 100644 --- a/apps/web/src/app/create/worksheets/addition/__tests__/tenFrames.test.ts +++ b/apps/web/src/app/create/worksheets/addition/__tests__/tenFrames.test.ts @@ -333,4 +333,262 @@ describe('Ten-frames rendering', () => { expect(firstPage).toContain('showTenFrames: false') }) }) + + describe('Mixed mode operator-specific scaffolding', () => { + it('should apply additionDisplayRules to addition problems in mixed mode', () => { + const config: WorksheetConfig = { + version: 4, + mode: 'mastery', + problemsPerPage: 4, + cols: 2, + pages: 1, + total: 4, + rows: 2, + orientation: 'portrait', + name: 'Test Student', + date: '2025-11-10', + seed: 12345, + fontSize: 12, + digitRange: { min: 2, max: 2 }, + operator: 'mixed', + pAnyStart: 1.0, + pAllStart: 0, + // Default display rules (not used for operator-specific problems) + displayRules: { + carryBoxes: 'never', + answerBoxes: 'never', + placeValueColors: 'never', + tenFrames: 'never', + problemNumbers: 'always', + cellBorders: 'always', + borrowNotation: 'never', + borrowingHints: 'never', + }, + // Operator-specific rules + additionDisplayRules: { + carryBoxes: 'whenRegrouping', + answerBoxes: 'always', + placeValueColors: 'always', + tenFrames: 'whenRegrouping', // ← Addition should show ten-frames + problemNumbers: 'always', + cellBorders: 'always', + borrowNotation: 'never', + borrowingHints: 'never', + }, + subtractionDisplayRules: { + carryBoxes: 'never', + answerBoxes: 'always', + placeValueColors: 'never', + tenFrames: 'never', // ← Subtraction should NOT show ten-frames + problemNumbers: 'always', + cellBorders: 'always', + borrowNotation: 'whenRegrouping', + borrowingHints: 'never', + }, + interpolate: false, + page: { wIn: 8.5, hIn: 11 }, + margins: { left: 0.5, right: 0.5, top: 0.5, bottom: 0.5 }, + } as any // Cast to any to allow operator-specific rules + + const problems: WorksheetProblem[] = [ + { operator: 'add', a: 45, b: 27 }, // Addition with regrouping + { operator: 'sub', minuend: 52, subtrahend: 18 }, // Subtraction with borrowing + ] + + const typstPages = generateTypstSource(config, problems) + const firstPage = typstPages[0] + + // Should contain both showTenFrames: true and showTenFrames: false + expect(firstPage).toContain('showTenFrames: true') // Addition + expect(firstPage).toContain('showTenFrames: false') // Subtraction + + // Verify operator assignment (Typst uses "+" and "−" display characters) + expect(firstPage).toContain('operator: "+"') + expect(firstPage).toContain('operator: "−"') + }) + + it('should apply subtractionDisplayRules to subtraction problems in mixed mode', () => { + const config: WorksheetConfig = { + version: 4, + mode: 'mastery', + problemsPerPage: 2, + cols: 1, + pages: 1, + total: 2, + rows: 2, + orientation: 'portrait', + name: 'Test Student', + date: '2025-11-10', + seed: 12345, + fontSize: 12, + digitRange: { min: 2, max: 2 }, + operator: 'mixed', + pAnyStart: 1.0, + pAllStart: 0, + displayRules: { + carryBoxes: 'never', + answerBoxes: 'never', + placeValueColors: 'never', + tenFrames: 'never', + problemNumbers: 'always', + cellBorders: 'always', + borrowNotation: 'never', + borrowingHints: 'never', + }, + additionDisplayRules: { + carryBoxes: 'never', + answerBoxes: 'always', + placeValueColors: 'never', + tenFrames: 'never', // ← Addition: no ten-frames + problemNumbers: 'always', + cellBorders: 'always', + borrowNotation: 'never', + borrowingHints: 'never', + }, + subtractionDisplayRules: { + carryBoxes: 'never', + answerBoxes: 'always', + placeValueColors: 'always', + tenFrames: 'whenRegrouping', // ← Subtraction: show ten-frames when borrowing + problemNumbers: 'always', + cellBorders: 'always', + borrowNotation: 'whenRegrouping', + borrowingHints: 'whenRegrouping', + }, + interpolate: false, + page: { wIn: 8.5, hIn: 11 }, + margins: { left: 0.5, right: 0.5, top: 0.5, bottom: 0.5 }, + } as any + + const problems: WorksheetProblem[] = [ + { operator: 'sub', minuend: 52, subtrahend: 18 }, // Subtraction with borrowing + ] + + const typstPages = generateTypstSource(config, problems) + const firstPage = typstPages[0] + + // Subtraction with borrowing should show ten-frames + expect(firstPage).toContain('showTenFrames: true') + expect(firstPage).toContain('showBorrowNotation: true') + expect(firstPage).toContain('showBorrowingHints: true') + }) + + it('should handle subtraction problems with operator "sub" correctly', () => { + // This test verifies the fix for the Unicode operator bug + const config: WorksheetConfig = { + version: 4, + mode: 'mastery', + problemsPerPage: 2, + cols: 1, + pages: 1, + total: 2, + rows: 2, + orientation: 'portrait', + name: 'Test Student', + date: '2025-11-10', + seed: 12345, + fontSize: 12, + digitRange: { min: 2, max: 2 }, + operator: 'mixed', + pAnyStart: 1.0, + pAllStart: 0, + displayRules: { + carryBoxes: 'never', + answerBoxes: 'always', + placeValueColors: 'never', + tenFrames: 'never', + problemNumbers: 'always', + cellBorders: 'always', + borrowNotation: 'never', + borrowingHints: 'never', + }, + additionDisplayRules: { + carryBoxes: 'always', + answerBoxes: 'always', + placeValueColors: 'always', + tenFrames: 'always', + problemNumbers: 'always', + cellBorders: 'always', + borrowNotation: 'never', + borrowingHints: 'never', + }, + subtractionDisplayRules: { + carryBoxes: 'never', + answerBoxes: 'always', + placeValueColors: 'always', + tenFrames: 'always', // ← Should show for subtraction + problemNumbers: 'always', + cellBorders: 'always', + borrowNotation: 'always', + borrowingHints: 'always', + }, + interpolate: false, + page: { wIn: 8.5, hIn: 11 }, + margins: { left: 0.5, right: 0.5, top: 0.5, bottom: 0.5 }, + } as any + + const problems: WorksheetProblem[] = [ + { operator: 'sub', minuend: 52, subtrahend: 18 }, // operator: 'sub' (alphanumeric) + { operator: 'add', a: 45, b: 27 }, // operator: 'add' (alphanumeric) + ] + + const typstPages = generateTypstSource(config, problems) + const firstPage = typstPages[0] + + // Both problems should show scaffolding (not zero scaffolding bug) + const tenFramesTrueMatches = firstPage.match(/showTenFrames: true/g) + expect(tenFramesTrueMatches?.length).toBe(2) // Both problems + + // Verify operators are correctly set (Typst uses "+" and "−" display characters) + expect(firstPage).toContain('operator: "−"') // Subtraction + expect(firstPage).toContain('operator: "+"') // Addition + }) + + it('should fallback to default displayRules when operator-specific rules are missing', () => { + const config: WorksheetConfig = { + version: 4, + mode: 'mastery', + problemsPerPage: 2, + cols: 1, + pages: 1, + total: 2, + rows: 2, + orientation: 'portrait', + name: 'Test Student', + date: '2025-11-10', + seed: 12345, + fontSize: 12, + digitRange: { min: 2, max: 2 }, + operator: 'mixed', + pAnyStart: 1.0, + pAllStart: 0, + // Only default rules, no operator-specific rules + displayRules: { + carryBoxes: 'whenRegrouping', + answerBoxes: 'always', + placeValueColors: 'always', + tenFrames: 'whenRegrouping', + problemNumbers: 'always', + cellBorders: 'always', + borrowNotation: 'whenRegrouping', + borrowingHints: 'never', + }, + interpolate: false, + page: { wIn: 8.5, hIn: 11 }, + margins: { left: 0.5, right: 0.5, top: 0.5, bottom: 0.5 }, + } + + const problems: WorksheetProblem[] = [ + { operator: 'add', a: 45, b: 27 }, // Has regrouping + { operator: 'sub', minuend: 52, subtrahend: 18 }, // Has borrowing + ] + + const typstPages = generateTypstSource(config, problems) + const firstPage = typstPages[0] + + // Both should use default rules and show scaffolding + const tenFramesTrueMatches = firstPage.match(/showTenFrames: true/g) + expect(tenFramesTrueMatches?.length).toBe(2) // Both problems + }) + }) }) diff --git a/apps/web/src/app/create/worksheets/addition/components/config-panel/AllSkillsModal.tsx b/apps/web/src/app/create/worksheets/addition/components/config-panel/AllSkillsModal.tsx index ec13b230..f8ec2850 100644 --- a/apps/web/src/app/create/worksheets/addition/components/config-panel/AllSkillsModal.tsx +++ b/apps/web/src/app/create/worksheets/addition/components/config-panel/AllSkillsModal.tsx @@ -463,11 +463,7 @@ export function AllSkillsModal({ { value: 'available', label: 'Available', count: availableSkills.length }, { value: 'locked', label: 'Locked', count: lockedSkills.length }, ].map((tab) => ( - +