# Quality & Adversarial Review Report: Milestone M2

- **Reviewer**: `reviewer_m2_progression_1`
- **Roles**: reviewer, critic
- **Target**: Milestone M2 Deliverables (LevelProgressionService & Death Penalty Engine)
- **Implementer**: `worker_m2_progression_1`
- **Parent**: `orchestrator_4` (`6f4a2aa2-4315-4660-8cb7-8352a7220c95`)
- **Date**: 2026-10-01T03:07:00Z
- **Verdict**: **REQUEST_CHANGES**

---

## 1. Review Summary

The deliverables for Milestone M2 demonstrate commendable engineering craft:
- All models in `server/world/level_progression_types.py` use `@dataclass(slots=True, frozen=True)` with backward-compatible property aliases.
- Strict typing is enforced (Mypy passes with 0 errors across all 4 files).
- Code length hygiene constraints are satisfied (all logic files $\le 337$ lines, under the 350 soft cap; all functions $\le 50$ lines).
- Existing unit tests (33) and E2E tests (22 selected, 47 overall) pass cleanly.
- No integrity violations or cheating patterns (no hardcoded test outputs, no fake stubs, no simulated logs).

However, **adversarial stress testing revealed two Major Functional Defects** that directly violate authoritative system requirements:
1. **Level Cap at 100 Cumulative EXP Overflow**: When a character transitions from Level 99 to Level 100 with an EXP gain larger than the remaining EXP to 100, `player.cumulative_exp` overflows the canonical Level 100 benchmark cap (`24,285,477,315`) by up to hundreds of thousands or millions of EXP. This directly violates the directive: *"Level cap at 100: zero overflow past Level 100 cumulative EXP."*
2. **Level 100 Death Penalty Event & State Swallowing**: In `apply_death_penalty()`, an early return for `player.level >= 100` bypasses incrementing `player.deaths_count` and bypasses notifying registered `_death_listeners`. Consequently, combat deaths at Level 100 are completely swallowed: telemetry, death counters, respawn hooks, and hardcore perma-death triggers never fire.
3. **Negative Base EXP Sanitation (Minor/Edge Case)**: `award_monster_exp()` does not sanitize `raw_exp >= 0`, allowing negative base EXP awards to reduce player current EXP.

Because Requirement 2 explicitly requires *"zero overflow past Level 100 cumulative EXP"*, the verdict is **REQUEST_CHANGES**.

---

## 2. Findings

### [Major] Finding 1: Cumulative EXP Overflows Canonical Cap at Level 100
- **Where**: `server/world/level_progression_service.py:253` and `server/world/level_progression_service.py:111`
- **What**: In `_apply_exp_gain()`, when a character advances to Level 100, `cumulative_exp` is updated as `player.cumulative_exp + exp_awarded`. Although `curr_exp` is correctly clamped to 0, `cumulative_exp` is NOT clamped to `self.get_benchmark(100).cumulative_exp`. For example, a Level 99 player needing 10 EXP who defeats a boss awarding 1,000,000 EXP ends up with `cumulative_exp = 24,286,477,305` instead of the cap `24,285,477,315` (an overflow of 999,990 EXP). Additionally, `set_player_state()` calculates `cum_exp = (bench.cumulative_exp if bench else 0) + current_exp` without clamping when `valid_level >= 100`.
- **Why**: Violates the authoritative directive: *"Level cap at 100: zero overflow past Level 100 cumulative EXP."* Corrupts progression telemetry and leaderboards.
- **Suggestion**:
  1. In `_apply_exp_gain()`:
     ```python
     b100 = self.get_benchmark(100)
     max_cum = b100.cumulative_exp if b100 else 0
     new_cum = min(max_cum, player.cumulative_exp + exp_awarded) if curr_lvl >= 100 else player.cumulative_exp + exp_awarded
     ```
  2. In `set_player_state()`:
     ```python
     if valid_level >= 100:
         cum_exp = bench.cumulative_exp if bench else 0
     ```

### [Major] Finding 2: Level 100 Deaths Swallow Listeners and Fail to Increment `deaths_count`
- **Where**: `server/world/level_progression_service.py:298-300`
- **What**: When `apply_death_penalty()` is called on a Level 100 character:
  ```python
  if player.level >= 100:
      return self._build_death_result(player, 0, 0.0, player.current_exp)
  ```
  This early return exits before updating `deaths_count = player.deaths_count + 1` and before executing `for listener in self._death_listeners: listener(result)`.
- **Why**: When a Level 100 player suffers fatal damage in combat, `deaths_count` remains unchanged (0 deaths recorded), and downstream listeners (zone evacuation, respawn routing, hardcore death penalty listeners) are completely blind to the death.
- **Suggestion**:
  Remove the early return. Since `get_death_penalty_ratio(100) == 0.0`, standard death calculation naturally results in `nominal_loss = 0`, `exp_lost = 0`, `new_exp = 0`. Allowing the method to proceed increments `deaths_count` and triggers `_death_listeners` as intended.

### [Minor] Finding 3: Lack of Input Sanitization for Non-Positive EXP
- **Where**: `server/world/level_progression_service.py:151-158`
- **What**: If an external caller or malicious packet supplies `base_exp < 0`, `raw_exp * gap_mult` produces a negative award, decrementing `current_exp` and `cumulative_exp`.
- **Why**: Combat kill rewards should be strictly non-negative.
- **Suggestion**: Clamp `raw_exp = max(0, raw_exp)`.

---

## 3. Verified Claims

| Claim from Worker M2 | Verification Method | Status | Notes |
|---|---|---|---|
| Immutable DTOs with `@dataclass(slots=True, frozen=True)` | `python -c "..."` mutating attribute | **PASS** | Raises `FrozenInstanceError` |
| Strict typing with 0 errors | `python -m mypy ...` | **PASS** | 0 errors across 4 files |
| Code length $\le 350-500$ lines, functions $\le 50$ lines | `check_code_and_doc_hygiene.py --strict` | **PASS** | Max file 337 lines, 0 functions > 50 lines |
| Level gap decay formula ($\Delta \le 5 \implies 1.0$, $\Delta = 10 \implies \le 0.05$) | `pytest test_level_progression_service.py -k TestLevelGapDecayFormula` | **PASS** | $\exp(-3.0) \approx 0.04978 \le 0.05$ |
| Tiered death penalty (0%, 5%, 10%, 15%, 25%, 0%) | `pytest test_level_progression_service.py -k TestTieredDeathPenalty` | **PASS** | Exact ratios across all brackets |
| Safe floor at 0% (never de-levels) | Unit tests & manual inspection | **PASS** | `de_leveled = False`, `new_exp >= 0` |
| CombatEngine fatal damage hook | `test_combat_fatal_damage_triggers_death_penalty` | **PASS** | Functional callback dispatch |
| Level cap at 100 zero overflow | Adversarial stress test | **FAIL** | Overflows by up to 999,990 EXP |

---

## 4. Adversarial Challenges

### Challenge 1: Level 99 to 100 Over-Allocation Attack
- **Assumption Challenged**: Reaching level 100 truncates progress cleanly to the level 100 cap.
- **Attack Scenario**: Player is Level 99 with `current_exp = d99 - 10`. Player slays an endgame boss yielding 1,000,000 raw EXP.
- **Observed Behavior**: `info.cumulative_exp = 24,286,477,305`, while `b100.cumulative_exp = 24,285,477,315`. Overflow = 999,990 EXP.
- **Blast Radius**: Corrupts lifetime EXP telemetry, breaks leaderboards, violates system requirement.
- **Mitigation**: Clamp `cumulative_exp` to `b100.cumulative_exp` when `curr_lvl >= 100`.

### Challenge 2: Immortal Level 100 Telemetry Black Hole
- **Assumption Challenged**: All player deaths trigger death penalty hooks and update death telemetry.
- **Attack Scenario**: Level 100 player is killed by fatal boss strike in `CombatEngine`.
- **Observed Behavior**: `deaths_count` remains 0; zero callbacks dispatched to `_death_listeners`.
- **Blast Radius**: Level 100 players bypass hardcore permadeath exile; death telemetry is completely missing.
- **Mitigation**: Remove early return in `apply_death_penalty()`; let 0% ratio update `deaths_count` and dispatch event.

### Challenge 3: Negative EXP Injection
- **Assumption Challenged**: Monster defeat always increases or maintains character experience.
- **Attack Scenario**: Calling `award_monster_exp(pid, 10, base_exp=-100)`.
- **Observed Behavior**: Player lost 1 EXP (`exp_awarded = -1`, `current_exp` dropped from 100 to 99).
- **Blast Radius**: Potential exploit or glitch where debuffs or negative packet data drain player EXP.
- **Mitigation**: Clamp `raw_exp = max(0, raw_exp)`.

---

## 5. Required Actions for Implementer

1. Update `server/world/level_progression_service.py`:
   - Clamp `cumulative_exp` to Level 100 benchmark cumulative EXP in `_apply_exp_gain()` and `set_player_state()`.
   - Remove early return at line 299 in `apply_death_penalty()`, allowing `deaths_count` to increment and `_death_listeners` to receive `DeathPenaltyResult(..., exp_lost=0, penalty_ratio=0.0)`.
   - Add `raw_exp = max(0, raw_exp)` in `_resolve_award_args()`.
2. Update `tests/unit/test_level_progression_service.py`:
   - Add unit test verifying Level 99 -> 100 transition caps `cumulative_exp` exactly at `b100.cumulative_exp` with 0 overflow.
   - Add unit test verifying death at Level 100 increments `deaths_count` and triggers `_death_listeners`.
   - Add unit test verifying negative base EXP does not decrement EXP.