# Phase 3 Review Fixes - Summary

All fixes from the Phase 3 review have been successfully applied to the Python OR-Tools CP-SAT solver.

## HIGH PRIORITY FIXES (All 6 Fixed ✅)

### FIX H4: C1 uses invalid OnlyEnforceIf pattern ✅
**File:** `constraint_builder.py` line 71

**Issue:** The constraint `model.Add(sum(employee_vars) <= 1).OnlyEnforceIf(employee_vars)` was incorrect. `OnlyEnforceIf` with a list of BoolVars means the constraint only applies when ALL vars are true, which defeats the purpose.

**Fix:** Removed `.OnlyEnforceIf(employee_vars)`. C1 is now unconditional:
```python
model.Add(sum(employee_vars) <= 1)
```

### FIX H2: Missing filled[s] BoolVar — coverage overcounts ✅
**File:** `objective_builder.py` lines 44-57

**Issue:** Coverage weight was added for every employee variable of every shift. A shift with 10 eligible employees got 10x the coverage incentive vs a shift with 1 eligible employee.

**Fix:** Created `filled[s]` BoolVar for each shift using `AddMaxEquality`:
```python
filled = model.NewBoolVar(f"filled_{shift_id}")
model.AddMaxEquality(filled, shift_vars)
objective_terms.append(filled * COVERAGE_WEIGHT)
```

### FIX H3: Fairness objective rewards under-allocation ✅
**File:** `objective_builder.py` lines 86-122

**Issue:** Linear penalty rewarded negative deviation (under-allocation).

**Fix:** Implemented proper absolute-value linearization:
```python
abs_dev = model.NewIntVar(0, max_hours_centi, f"abs_dev_{emp_id}")
model.Add(abs_dev >= total_hours - hours_requested_centi)
model.Add(abs_dev >= hours_requested_centi - total_hours)
objective_terms.append(abs_dev * (-fairness_weight))
```

### FIX H5: C4 opening/closing uses wrong detection ✅
**File:** `constraint_builder.py` lines 106-124

**Issue:** Used time-based detection (before 8am = opening, after 10pm = closing). SDD says: "if shift.minRoleId <= 3, only employees with role <= 3 can be assigned."

**Fix:** Changed to minRoleId-based detection:
```python
if min_role_id <= 3:
    if emp_role > 3:
        model.Add(x[(emp_id, shift_id)] == 0)
```

### FIX H6: Overnight availability only checks start day ✅
**File:** `constraint_builder.py` lines 126-161

**Issue:** For overnight shifts (endTime < startTime), code only checked start day availability.

**Fix:** For overnight shifts, check both start day and next day availability:
```python
if is_overnight:
    start_day_avail = [a for a in avail_list if a["dayOfWeek"] == dow]
    next_day = (dow % 7) + 1
    next_day_avail = [a for a in avail_list if a["dayOfWeek"] == next_day]

    start_ok = any(...)
    end_ok = any(...)

    if not start_ok or not end_ok:
        model.Add(x[(emp_id, shift_id)] == 0)
```

### FIX H1: IIS analyzer should use assumption literals ✅
**File:** `infeasibility_analyzer.py`

**Issue:** Current implementation was a heuristic stub.

**Fix:** Implemented CP-SAT assumption-literal strategy:
1. For each shift, create assumption literal representing "this shift must be filled"
2. Add "shift must have exactly 1 employee" constraint guarded by assumption
3. Call `solver.SolveWithAssumptions(assumptions)`
4. If infeasible, use `solver.SufficientAssumptionsForInfeasibility()` to get minimal conflicting set
5. Fall back to heuristic analysis if timeout (10s budget)

Returns:
```python
{
    "method": "assumption_literals",
    "conflictingShiftIds": [list of shift IDs],
    "explanation": "N shifts cannot all be filled simultaneously"
}
```

## MEDIUM PRIORITY FIXES (All 5 Fixed ✅)

### FIX M1: Remove duplicate locked shift handling ✅
**File:** `model_builder.py` lines 44-50

**Issue:** Locked shift variable fixing in model_builder.py duplicated C2 in constraint_builder.py.

**Fix:** Removed duplicate code from model_builder.py. C2 in constraint_builder.py is the single source of truth.

### FIX M2: Labor cost should use actual cost in cents ✅
**File:** `objective_builder.py` lines 59-83

**Issue:** Used normalized rate instead of actual cost.

**Fix:** Calculate actual cost in cents:
```python
cost_cents = int(shift["durationHours"] * hourly_rate * 100)
penalty = cost_cents * labor_cost_weight // 100
objective_terms.append(x[(emp_id, shift_id)] * (-penalty))
```

### FIX M3: Daily OT should use per-day aggregate ✅
**File:** `objective_builder.py` lines 171-186

**Issue:** Checked if individual shifts exceeded 8h, not daily totals.

**Fix:** Group shifts by employee and date, create daily hour sum variables:
```python
daily_ot = model.NewIntVar(0, 10000, f"daily_ot_{emp_id}_{date}")
model.Add(daily_ot >= daily_total - daily_threshold_centi)
model.Add(daily_ot >= 0)
objective_terms.append(daily_ot * (-overtime_weight))
```

### FIX M5: C9 should handle cross-midnight overlap ✅
**File:** `constraint_builder.py` lines 217-245

**Issue:** Only checked same-day overlaps. Overnight shifts on day D can overlap with shifts on day D+1.

**Fix:** Added cross-midnight overlap detection:
```python
elif (date2 - date1).days == 1 and is_overnight1:
    overlaps = (end1 > start2)
elif (date1 - date2).days == 1 and is_overnight2:
    overlaps = (end2 > start1)
```

### FIX M8: objectiveValue reporting ✅
**File:** `result_formatter.py` line 44

**Issue:** Divided objective value by 100, treating it as centihours/cents.

**Fix:** Report raw objective value (dimensionless quality score):
```python
objective_value = solver.objective_value if status in ["OPTIMAL", "FEASIBLE"] else 0.0
result["objectiveValue"] = int(objective_value)
```

### FIX M6: Create complex_store.json ✅
**File:** `tests/fixtures/complex_store.json`

**Created:** Realistic fixture with:
- 20 employees (roles 1-5)
- 27 shifts across a week
- Mixed roles, varying availability
- Some time off
- 1 locked shift
- 2 overnight shifts

### FIX M7: Post-solve constraint validation ✅
**File:** `result_formatter.py`

**Added:** `validate_constraints()` function that checks all 11 hard constraints against extracted assignments:
```python
violations = validate_constraints(problem_data, assignments)
total_violations = sum(violations.values())
result["scorecard"]["constraintViolations"] = total_violations
```

## LOW PRIORITY FIXES (All 2 Fixed ✅)

### FIX L3: Remove unused imports ✅
**File:** `constraint_builder.py` line 7

**Fix:** Removed unused `time` import. Kept `datetime` (used in C9).

### FIX L1: Add __init__.py files ✅
**Files:** `userfrosting/solver/__init__.py`, `userfrosting/solver/tests/__init__.py`

**Fix:** Created empty `__init__.py` in solver directory. Tests directory already had one.

## Test Results

### All Tests Pass ✅
```
42 passed in 12.98s
```

### Self-Test Pass ✅
```
Self-test PASSED ✓
```

### New Tests Added
1. `test_overnight_shift_requires_next_day_availability` - FIX H6
2. `test_overnight_shift_with_full_availability` - FIX H6
3. `test_fairness_penalizes_under_allocation` - FIX H3
4. `test_coverage_counts_each_shift_once` - FIX H2
5. `test_constraint_validation_detects_violations` - FIX M7
6. `test_complex_store_fixture_solvable` - FIX M6
7. `test_objective_value_is_integer` - FIX M8

### Updated Tests
1. `test_c4_opening_closing_requires_role_3` - Updated to use minRoleId-based logic
2. `test_opening_closing_requires_role_3` - Updated to use minRoleId-based logic

## Summary

All 13 fixes have been successfully implemented:
- ✅ 6 HIGH priority fixes
- ✅ 5 MEDIUM priority fixes
- ✅ 2 LOW priority fixes

The solver now correctly implements:
1. Proper constraint logic (C1, C4, C5 overnight, C9 overnight)
2. Correct objective function (coverage, fairness, labor cost, daily OT)
3. Assumption-literal IIS analysis
4. Post-solve constraint validation
5. Clean code without duplication or unused imports

All existing tests continue to pass, and new tests validate the fixes.