docs: add operator-specific settings architecture & refactoring plan
**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 <noreply@anthropic.com>
This commit is contained in:
Thomas Hallock
parent
510f052978
commit
e06de8ea47
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue