From e06de8ea47a548139a542b3d7edfca1b56678f83 Mon Sep 17 00:00:00 2001 From: Thomas Hallock Date: Mon, 17 Nov 2025 18:24:07 -0600 Subject: [PATCH] docs: add operator-specific settings architecture & refactoring plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Problem:** Mixed mastery mode uses operator-specific setting overrides that are easy to overlook when making changes. This caused the recent scaffolding bug and will get worse as we add more operators. **Created Documentation:** - OPERATOR_SPECIFIC_SETTINGS.md - Complete architecture guide - Explains current implementation (global vs operator-specific) - Documents recent scaffolding bug and root cause - Lists all components that need special handling - Provides 4-phase refactoring plan before adding new operators **Key Insights:** - Currently: 2 operators β†’ 2 optional override fields per setting - Future: 6+ operators β†’ 6+ override fields = exponential complexity - Hidden complexity: Not obvious from schema which fields have overrides - Bug prone: Every UI component needs conditional logic **Refactoring Plan:** 1. Phase 1: Documentation & audit (this commit) 2. Phase 2: Unified config model (explicit operator structure) 3. Phase 3: UI clarity (visual indicators, bulk actions) 4. Phase 4: Validation & testing **Documentation Graph:** - Linked from main README under "Additional Documentation" - Marked with ⚠️ [NEEDS REFACTORING] for visibility - Connected to Config Schema, Mastery, and Subtraction docs **Next Steps:** - Audit ContentTab and DifficultyTab for operator-specific handling - Add integration tests for mixed mode settings - Complete refactoring BEFORE adding multiplication/division πŸ€– Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../worksheets/OPERATOR_SPECIFIC_SETTINGS.md | 382 ++++++++++++++++++ apps/web/src/app/create/worksheets/README.md | 1 + 2 files changed, 383 insertions(+) create mode 100644 apps/web/src/app/create/worksheets/OPERATOR_SPECIFIC_SETTINGS.md diff --git a/apps/web/src/app/create/worksheets/OPERATOR_SPECIFIC_SETTINGS.md b/apps/web/src/app/create/worksheets/OPERATOR_SPECIFIC_SETTINGS.md new file mode 100644 index 00000000..bc052e5c --- /dev/null +++ b/apps/web/src/app/create/worksheets/OPERATOR_SPECIFIC_SETTINGS.md @@ -0,0 +1,382 @@ +# Operator-Specific Settings Architecture + +**Status:** ⚠️ Current Implementation (Needs Refactoring) +**Complexity Level:** High - Easy to overlook when making changes +**Urgency:** High - More operators coming soon (multiplication, division, fractions, etc.) + +## Overview + +Mixed mastery mode uses **operator-specific settings** that override global settings. This hidden complexity has caused bugs and will become increasingly problematic as we add more operators. + +**Current operators:** +- Addition +- Subtraction + +**Planned operators:** +- Multiplication +- Division +- Fractions +- Decimals +- Mixed operations + +## The Problem + +### Current Architecture + +When `mode === 'mastery' && operator === 'mixed'`, the system uses operator-specific overrides: + +```typescript +// Global settings (apply to single-operator mode) +displayRules: DisplayRules + +// Operator-specific overrides (mastery+mixed mode only) +additionDisplayRules?: DisplayRules +subtractionDisplayRules?: DisplayRules +``` + +**Settings that are operator-specific in mixed mode:** +1. **Display Rules** - Scaffolding (answer boxes, carry boxes, ten frames, etc.) +2. **Digit Range** - Number of digits per operator +3. **Regrouping Config** - pAnyStart, pAllStart, interpolate +4. **Current Skill** - Tracks progress separately per operator + +### Why This Is Problematic + +1. **Hidden Complexity** + - Not obvious from config schema which fields have operator-specific variants + - Easy to forget when updating UI components + - No clear pattern for when to use global vs operator-specific + +2. **Code Duplication** + - Every UI component that modifies settings needs conditional logic + - Example: ScaffoldingTab must check `mode === 'mastery' && operator === 'mixed'` + - Same logic repeated across multiple components + +3. **Bug Prone** + - Recent bug: Scaffolding changes ignored in mixed mode (fixed in commit 40389f39) + - Root cause: ScaffoldingTab only updated global `displayRules`, not operator-specific + - Will happen again with new operators unless we refactor + +4. **Scaling Problem** + - Currently 2 operators β†’ 2 optional override fields + - With 6 operators β†’ 6 optional override fields per setting + - Exponential complexity as operators grow + +### Concrete Example: The Bug We Just Fixed + +**User Action:** In mastery+mixed mode, change answer boxes from "always" β†’ "never" + +**Expected:** Both addition AND subtraction problems hide answer boxes + +**What Happened:** +1. ScaffoldingTab updated `displayRules` (global) +2. Typst generator checked for operator-specific rules first: + ```typescript + if (p.operator === 'add' && config.additionDisplayRules) { + rulesForProblem = config.additionDisplayRules // ← Still has old value! + } + ``` +3. User's change ignored because old operator-specific rules took precedence + +**Fix Required:** +```typescript +// In ScaffoldingTab - must update ALL three fields +if (isMasteryMixed) { + onChange({ + displayRules: newRules, // Global + additionDisplayRules: newRules, // Operator-specific + subtractionDisplayRules: newRules // Operator-specific + }) +} +``` + +## Current Implementation Details + +### Config Schema + +```typescript +// V4 Mastery Mode Schema +export const additionConfigV4MasteryModeSchema = z.object({ + mode: z.literal('mastery'), + + // Global display rules + displayRules: displayRulesSchema, + + // Operator-specific overrides (mixed mode only) + additionDisplayRules: displayRulesSchema.optional(), + subtractionDisplayRules: displayRulesSchema.optional(), + + // Skill tracking (always operator-specific in mastery) + currentAdditionSkillId: z.string().optional(), + currentSubtractionSkillId: z.string().optional(), +}) +``` + +### Components That Need Special Handling + +1. **ScaffoldingTab** βœ… Fixed (commit 40389f39) + - Updates all operator-specific display rules when in mixed mode + +2. **ContentTab** ⚠️ Needs Review + - Controls operator selection and digit range + - May need operator-specific digit range handling + +3. **DifficultyTab** ⚠️ Needs Review + - Controls regrouping probabilities + - May need operator-specific regrouping config + +4. **MasteryModePanel** βœ… Already correct + - Manages skill selection per operator + - Already operator-aware + +### Code Locations + +**Schema Definition:** +- `src/app/create/worksheets/config-schemas.ts` (lines 536-650) + +**Typst Generation:** +- `src/app/create/worksheets/typstGenerator.ts` (lines 69-80) +- Checks for operator-specific display rules + +**Problem Generation:** +- `src/app/create/worksheets/generatePreview.ts` (lines 89-150) +- Uses skill-specific configs for mixed mode + +**UI Components:** +- `src/app/create/worksheets/components/config-sidebar/ScaffoldingTab.tsx` +- `src/app/create/worksheets/components/config-sidebar/ContentTab.tsx` +- `src/app/create/worksheets/components/config-sidebar/DifficultyTab.tsx` + +## Refactoring Plan + +### Phase 1: Documentation & Audit (Immediate) + +**Goals:** +- Document current behavior (this file) +- Audit all components for operator-specific handling +- Create test cases for mixed mode settings + +**Tasks:** +- [x] Document architecture in this file +- [ ] Audit ContentTab for operator-specific handling +- [ ] Audit DifficultyTab for operator-specific handling +- [ ] Add integration tests for mixed mode settings +- [ ] Link this doc from main README + +### Phase 2: Unified Config Model (Before Adding New Operators) + +**Goals:** +- Replace optional operator-specific fields with structured model +- Make operator-specificity explicit in schema +- Single source of truth for which settings are operator-specific + +**Proposed Schema:** + +```typescript +// BEFORE (Current - Implicit) +interface MasteryConfig { + displayRules: DisplayRules + additionDisplayRules?: DisplayRules // Hidden override + subtractionDisplayRules?: DisplayRules // Hidden override +} + +// AFTER (Proposed - Explicit) +interface MasteryConfig { + operatorMode: 'single' | 'mixed' + + // Single operator mode + singleOperatorSettings?: { + operator: 'addition' | 'subtraction' + displayRules: DisplayRules + digitRange: DigitRange + regroupingConfig: RegroupingConfig + } + + // Mixed operator mode + mixedOperatorSettings?: { + operators: { + addition: OperatorSettings + subtraction: OperatorSettings + multiplication?: OperatorSettings // Future + division?: OperatorSettings // Future + } + } +} + +interface OperatorSettings { + enabled: boolean + displayRules: DisplayRules + digitRange: DigitRange + regroupingConfig: RegroupingConfig + currentSkillId?: string +} +``` + +**Benefits:** +- Clear separation between single and mixed modes +- Easy to add new operators (just add to `operators` object) +- No hidden overrides - structure makes it obvious +- Type-safe access to operator-specific settings + +**Migration Strategy:** +1. Add new schema alongside old (dual-write) +2. Update components to read from new schema +3. Add migration from V4 β†’ V5 +4. Deprecate old operator-specific fields + +### Phase 3: UI Clarity (With Phase 2) + +**Goals:** +- Make it visually obvious when settings are operator-specific +- Show which operator settings apply to +- Allow easy bulk updates ("Apply to All Operators") + +**Proposed UI Changes:** + +1. **Scaffolding Tab - Mixed Mode View:** + ``` + β”Œβ”€ Scaffolding ─────────────────┐ + β”‚ Mode: Mixed (Addition + Sub) β”‚ + β”‚ β”‚ + β”‚ β”Œβ”€ Addition Settings ────┐ β”‚ + β”‚ β”‚ Answer Boxes: Always β”‚ β”‚ + β”‚ β”‚ Carry Boxes: Never β”‚ β”‚ + β”‚ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β”‚ + β”‚ β”‚ + β”‚ β”Œβ”€ Subtraction Settings ─┐ β”‚ + β”‚ β”‚ Answer Boxes: Never β”‚ β”‚ + β”‚ β”‚ Borrow Notation: Alwaysβ”‚ β”‚ + β”‚ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β”‚ + β”‚ β”‚ + β”‚ [Apply Same to All Ops] [↓] β”‚ + β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ + ``` + +2. **Visual Indicators:** + - Operator icon badges (βž• βž– βœ–οΈ βž—) next to settings + - Different background colors per operator + - Clear "Mixed Mode" banner at top + +3. **Bulk Actions:** + - "Apply to All Operators" button + - "Copy from Addition to Subtraction" action + - "Reset to Skill Defaults" per operator + +### Phase 4: Validation & Testing (Ongoing) + +**Test Coverage:** + +1. **Unit Tests** + ```typescript + describe('Mixed Mode Settings', () => { + it('updates all operator-specific rules when scaffolding changes', () => { + // Test that ScaffoldingTab updates addition AND subtraction rules + }) + + it('preserves operator-specific rules during migrations', () => { + // Test V4β†’V5 migration keeps operator settings + }) + }) + ``` + +2. **Integration Tests** + ```typescript + describe('Mixed Mode Preview', () => { + it('uses correct display rules per operator in preview', () => { + // Set different rules for addition vs subtraction + // Verify preview shows correct scaffolding per problem type + }) + }) + ``` + +3. **E2E Tests** + ```typescript + describe('Mixed Mode User Flow', () => { + it('allows independent scaffolding per operator', () => { + // User sets addition with answer boxes, subtraction without + // Download PDF and verify both operators rendered correctly + }) + }) + ``` + +## Adding New Operators + +### Current Process (Before Refactoring) + +⚠️ **High Risk of Bugs** - Easy to miss steps + +1. Add operator to schema enum +2. Add `{operator}DisplayRules?: DisplayRules` field +3. Update every component that modifies display rules: + - ScaffoldingTab βœ… + - ContentTab ⚠️ + - DifficultyTab ⚠️ + - Any other settings tabs +4. Update Typst generator to check for operator-specific rules +5. Update problem generator to handle new operator +6. Update migrations to preserve new operator fields +7. Add tests for new operator + +**Miss any step β†’ Settings silently ignored (like our recent bug)** + +### Future Process (After Refactoring) + +βœ… **Type-Safe & Explicit** + +1. Add operator to `OperatorType` enum: + ```typescript + type OperatorType = 'addition' | 'subtraction' | 'multiplication' + ``` + +2. Schema automatically supports it: + ```typescript + mixedOperatorSettings: { + operators: { + multiplication: OperatorSettings // Just add this + } + } + ``` + +3. UI components automatically show new operator (data-driven) +4. Type system enforces all operator-specific settings are handled +5. Tests automatically cover new operator via parameterized tests + +## Migration Checklist + +Before adding any new operator, complete these refactoring phases: + +- [ ] Phase 1: Documentation & Audit + - [ ] Document current architecture (this file) + - [ ] Audit all components + - [ ] Add integration tests + - [ ] Link from main README + +- [ ] Phase 2: Unified Config Model + - [ ] Design new schema structure + - [ ] Implement V4β†’V5 migration + - [ ] Update components to use new schema + - [ ] Deprecate old operator-specific fields + +- [ ] Phase 3: UI Clarity + - [ ] Design mixed mode UI mockups + - [ ] Implement operator-specific views + - [ ] Add bulk action buttons + - [ ] User testing for clarity + +- [ ] Phase 4: Validation & Testing + - [ ] Unit tests for settings updates + - [ ] Integration tests for mixed mode + - [ ] E2E tests for user flows + - [ ] Performance testing with 6+ operators + +## Related Documentation + +- [Config Schema Guide](./CONFIG_SCHEMA_GUIDE.md) - Full config schema documentation +- [Mastery System Plan](./MASTERY_SYSTEM_PLAN.md) - Overall mastery system architecture +- [Problem Generation Architecture](./PROBLEM_GENERATION_ARCHITECTURE.md) - How problems are generated +- [Subtraction and Operator Plan](./SUBTRACTION_AND_OPERATOR_PLAN.md) - Original operator support plan + +## Recent Changes + +- **2025-01-17**: Fixed scaffolding settings ignored in mastery+mixed mode (commit 40389f39) +- **2025-01-17**: Created this documentation to prevent future bugs diff --git a/apps/web/src/app/create/worksheets/README.md b/apps/web/src/app/create/worksheets/README.md index 8578d6a2..00eaeb2c 100644 --- a/apps/web/src/app/create/worksheets/README.md +++ b/apps/web/src/app/create/worksheets/README.md @@ -45,6 +45,7 @@ - **Configuration Schema** - [`CONFIG_SCHEMA_GUIDE.md`](./CONFIG_SCHEMA_GUIDE.md) - Complete guide to worksheet configuration - **Smart Difficulty** - [`addition/SMART_DIFFICULTY_SPEC.md`](./addition/SMART_DIFFICULTY_SPEC.md) - Smart Mode progression spec - **Subtraction System** - [`SUBTRACTION_AND_OPERATOR_PLAN.md`](./SUBTRACTION_AND_OPERATOR_PLAN.md) - Subtraction implementation +- **⚠️ Operator-Specific Settings** - [`OPERATOR_SPECIFIC_SETTINGS.md`](./OPERATOR_SPECIFIC_SETTINGS.md) - **[NEEDS REFACTORING]** Mixed mode architecture & refactoring plan ## Key Concepts