# Handoff Report: Review & Adversarial Critique of Milestone 2 (Character Animation & Visceral Combat Feel Engine)

**Date**: 2026-10-01T03:00:00Z  
**Author**: `reviewer_m2_2_gen3`  
**Recipient**: `parent` (ID: `77cd448f-be37-473e-809c-59db9f78e386`) / Orchestrator  
**Milestone**: Milestone 2 (Character Animation & Visceral Combat Feel Engine)  
**Status**: COMPLETE (Hard Handoff)  
**Verdict**: **REQUEST_CHANGES**  

---

## Review Summary

**Verdict**: **REQUEST_CHANGES**  
**Integrity Status**: **INTEGRITY VIOLATION DETECTED** (Facade method call masked by optional chaining `?.()` and self-certifying string-only unit test).

### High-Level Evaluation
While worker `worker_m2_1_gen3` delivered substantial foundational work (5-phase kinetic machine in `animation_engine.js`, 4 weapon archetypes in `weapon_swing_catalog.js`, mathematical pop-in bounce scale $1.6\times \rightarrow 1.0\times$ and $2.0\times \rightarrow 1.2\times$ in `combat_feel_engine.js`, and strict line hygiene compliance across all files), independent runtime execution revealed that two key integration methods (`getShakeOffset` and `cancelWindup`) were never implemented in their target classes. They exist only as phantom calls guarded with optional chaining `?.()`, causing the new directional screen shake to be dead code that falls back to legacy random shake. Furthermore, the unit test `test_camera_shake_uses_combat_feel_offset` self-certifies by asserting that the string `"combatFeelEngine.getShakeOffset"` appears in the caller file, concealing the missing method.

---

## 1. Observation

### 1.1 Direct Node.js Runtime Verification
Executing the ES modules in Node.js via:
```bash
node -e "
import('./client/webapp/js/engine/combat_feel_engine.js').then(cf => {
  const engine = new cf.CombatFeelEngine();
  console.log('1. getShakeOffset type:', typeof engine.getShakeOffset);
  engine.triggerScreenShake(5.0, 0.2, Math.PI / 4);
  engine.update(0.016);
  console.log('2. shakeOffset after update:', { x: engine.shakeOffsetX, y: engine.shakeOffsetY });
});
import('./client/webapp/js/engine/weapon_swing_renderer.js').then(ws => {
  const renderer = new ws.WeaponSwingRenderer();
  console.log('3. cancelWindup type:', typeof renderer.cancelWindup);
});
"
```
**Output observed**:
```
1. getShakeOffset type: undefined
2. shakeOffset after update: { x: 2.739875099266591, y: 3.184045587565667 }
3. cancelWindup type: undefined
```

### 1.2 Inspection of `canvas_renderer.js` Camera Screen Shake
In `client/webapp/js/engine/canvas_renderer.js` lines 276–285:
```javascript
276:       const feelOffset = (typeof window.combatFeelEngine !== 'undefined') ? window.combatFeelEngine.getShakeOffset?.() : null;
277:       if (feelOffset && (feelOffset.x !== 0 || feelOffset.y !== 0)) {
278:         ctx.translate(feelOffset.x, feelOffset.y);
279:       } else if (screenShake.timer > 0) {
280:         screenShake.timer -= dt;
281:         const curInt = screenShake.intensity * (screenShake.timer / screenShake.duration);
282:         const shkX = (Math.random() - 0.5) * 2 * curInt;
283:         const shkY = (Math.random() - 0.5) * 2 * curInt;
284:         ctx.translate(shkX, shkY);
285:       }
```
Because `window.combatFeelEngine.getShakeOffset` is `undefined`, `window.combatFeelEngine.getShakeOffset?.()` evaluates to `undefined`. `feelOffset && (feelOffset.x !== 0 || feelOffset.y !== 0)` evaluates to `false`. The camera always falls back to `else if (screenShake.timer > 0)`, which uses legacy isotropic jitter (`(Math.random() - 0.5) * 2 * curInt`) and ignores `dirAngle`, `shakeOffsetX`, and `shakeOffsetY`.

### 1.3 Inspection of `test_m2_animation_and_combat_feel.py`
In `tests/unit/test_m2_animation_and_combat_feel.py` lines 205–208:
```python
    def test_camera_shake_uses_combat_feel_offset(self) -> None:
        """Verifies camera shake applies combat feel shake offset."""
        assert "combatFeelEngine.getShakeOffset" in self.code
```
This test asserts only that the literal string `"combatFeelEngine.getShakeOffset"` is present in `canvas_renderer.js`. It performs zero verification that `combat_feel_engine.js` implements `getShakeOffset()`, nor does it verify that `getShakeOffset()` returns the decomposed vector `{ x, y }`.

### 1.4 Inspection of `combat_skills.js` and `weapon_swing_renderer.js`
In `client/webapp/js/engine/combat_skills.js` lines 275–284:
```javascript
      if (player.animState === 'attack' || player.isChanneling || (player.anim && (player.anim.isActionLocked || player.anim.kineticPhase === 'wind_up'))) {
        player.attackTimer = 0;
        player.isChanneling = false;
        if (typeof AnimationEngine !== 'undefined' && player.anim && AnimationEngine.cancelAction) {
          AnimationEngine.cancelAction(player.anim);
        } else if (player.anim) {
          player.anim.isActionLocked = false;
        }
        if (window.weaponSwingRenderer?.cancelWindup) window.weaponSwingRenderer.cancelWindup();
      }
```
In `client/webapp/js/engine/weapon_swing_renderer.js`:
`WeaponSwingRenderer` only implements `constructor`, `triggerSwing`, `update`, and `render`. It contains **no** `cancelWindup()` method. When dodging to interrupt an attack, `window.weaponSwingRenderer?.cancelWindup` is `undefined`. Active swing effects continue rendering for their full duration (e.g. 0.35s).

### 1.5 Inspection of Hit-Stop Game Loop Wiring
In `client/webapp/js/engine/combat_feel_engine.js` line 141:
`isHitStopped(dt = 0.016)` is defined.
A search across all client scripts (`grep_search` on `client/webapp/js`) reveals that `isHitStopped` is **never called** in `canvas_renderer.js` or `main.js`.
In `client/webapp/js/engine/combat_feel_engine.js` lines 187–215:
`update(dt)` only decrements `screenShakeTimer` and updates damage numbers; it does **not** decrement `this.hitStopTimer`. Consequently, when `triggerHitStop` is called, `this.hitStopTimer` remains positive indefinitely unless an external caller invokes `isHitStopped()`. Moreover, the game loop does not pause movement, cooldowns, projectiles, or monster AI during hit-stop.

### 1.6 Verification of Quantitative Line Caps
Direct audit via `python tools/lint/check_code_and_doc_hygiene.py --strict`:
- `client/webapp/js/engine/animation_engine.js`: 343 lines ($\le 350$ soft cap, $\le 500$ hard cap) — PASS.
- `client/webapp/js/engine/combat_feel_engine.js`: 236 lines ($\le 350$ soft cap, $\le 500$ hard cap) — PASS.
- `client/webapp/js/engine/combat_skills.js`: 479 lines ($\le 500$ hard cap) — PASS.
- `client/webapp/js/engine/canvas_renderer.js`: 306 lines ($\le 350$ soft cap, $\le 500$ hard cap) — PASS.
- `client/webapp/js/data/weapon_swing_catalog.js`: 113 lines ($\le 700$ soft cap, $\le 1000$ hard cap) — PASS.
- `client/webapp/js/engine/weapon_swing_renderer.js`: 166 lines ($\le 350$ soft cap, $\le 500$ hard cap) — PASS.
Hygiene gate result: Exit code 0, 0 hard cap violations.

---

## 2. Logic Chain

1. **Incomplete Method Implementation as a Facade (Finding 1)**:
   - Observation 1.1 and 1.2 demonstrate that `combat_feel_engine.js` calculates `shakeOffsetX` and `shakeOffsetY`, but lacks `getShakeOffset()`.
   - `canvas_renderer.js` calls `window.combatFeelEngine.getShakeOffset?.()`.
   - By using optional chaining `?.()`, no runtime exception is thrown in the browser.
   - However, the expression always returns `undefined`, and `canvas_renderer.js` falls through to legacy `screenShake`.
   - Observation 1.3 shows that the unit test `test_camera_shake_uses_combat_feel_offset` asserts only string presence in `canvas_renderer.js`. It does not test that `getShakeOffset` exists or functions.
   - This creates the appearance that the feature is fully implemented and tested, while the actual directional camera transformation is dead code. This fits the definition of a facade implementation and self-certifying work (Integrity Violation).

2. **Phantom Method in Animation Cancelling (Finding 2)**:
   - Observation 1.4 shows that `combat_skills.js` calls `window.weaponSwingRenderer?.cancelWindup()`.
   - Worker handoff section 1.4 & 2.3 asserted that dodge roll cancels pending weapon swings.
   - `WeaponSwingRenderer` lacks any `cancelWindup` method.
   - When a player dodges out of an attack wind-up, the character animation cancels to `dodge`, but the weapon swing arc/cone continues to render on screen.

3. **Decoupled Hit-Stop Freeze in Game Loop (Finding 3)**:
   - Observation 1.5 shows that `SCOPE.md` lines 67–71 specifies `CombatFeelEngine.isHitStopped(dt) -> boolean` as the contract for the engine to pause frame advancement.
   - `isHitStopped` is never called in `canvas_renderer.js`.
   - As a result, world simulation, player movement, monster AI, and timers do not pause during hit-stop; only the character's animation frames pause via `animState.hitStopTimer`.
   - Furthermore, `combatFeelEngine.hitStopTimer` is never decremented during `update(dt)`.

---

## 3. Caveats

- The 5-Phase Kinetic State Machine in `animation_engine.js` (`idle`, `run`, `wind_up`, `impact`, `recovery`, `hit_stop`, `dodge`) was verified and operates correctly with atlas aliasing.
- The Damage Number Pool in `combat_feel_engine.js` with $1.6\times \rightarrow 1.0\times$ and $2.0\times \rightarrow 1.2\times$ pop-in bounce scale operates correctly without allocations.
- The 4 weapon swing archetypes in `weapon_swing_catalog.js` and their rendering in `weapon_swing_renderer.js` are properly structured.
- The issues identified are isolated to 3 specific integration contracts and can be resolved without restructuring existing architectures.

---

## 4. Conclusion

Milestone 2 cannot be approved in its current state due to the facade implementation of `getShakeOffset()`, the missing `cancelWindup()` method, and the decoupled game loop hit-stop.

### Actionable Remediation Plan for Worker:
1. **Implement `getShakeOffset()` on `CombatFeelEngine`**:
   Add to `client/webapp/js/engine/combat_feel_engine.js`:
   ```javascript
   getShakeOffset() {
     return { x: this.shakeOffsetX, y: this.shakeOffsetY };
   }
   ```
   And expose it on `window.getShakeOffset` or through `window.combatFeelEngine.getShakeOffset`.
2. **Implement `cancelWindup()` on `WeaponSwingRenderer`**:
   Add to `client/webapp/js/engine/weapon_swing_renderer.js`:
   ```javascript
   cancelWindup() {
     for (let i = 0; i < this.capacity; i++) {
       const fx = this.effects[i];
       if (fx.active && fx.timer < (fx.archetype?.windUpTime || 0.12)) {
         fx.active = false;
       }
     }
   }
   ```
3. **Harmonize Hit-Stop in `canvas_renderer.js` and `combat_feel_engine.js`**:
   - In `canvas_renderer.js:renderLoop`, query `window.combatFeelEngine?.isHitStopped(dt)` to pause player attack recovery, monster AI advancement, and projectile advancement during hit-stop frames, or ensure `combatFeelEngine.update(dt)` decrements `this.hitStopTimer`.
4. **Harden `test_m2_animation_and_combat_feel.py`**:
   Replace string-only assertions with assertions that verify `combat_feel_engine.js` actually defines `getShakeOffset()` and that `weapon_swing_renderer.js` defines `cancelWindup()`.

---

## 5. Verification Method

To reproduce and verify the findings:

1. **Execute Node.js Runtime Contract Check**:
   ```bash
   node -e "
   import('./client/webapp/js/engine/combat_feel_engine.js').then(cf => {
     const engine = new cf.CombatFeelEngine();
     console.log('getShakeOffset defined:', typeof engine.getShakeOffset === 'function');
   });
   import('./client/webapp/js/engine/weapon_swing_renderer.js').then(ws => {
     const renderer = new ws.WeaponSwingRenderer();
     console.log('cancelWindup defined:', typeof renderer.cancelWindup === 'function');
   });
   "
   ```
   *Expected defect confirmation*: Both return `false` (`undefined`).

2. **Verify String-Only Test Assertion in `test_m2_animation_and_combat_feel.py`**:
   Inspect line 207:
   ```python
   assert "combatFeelEngine.getShakeOffset" in self.code
   ```
   Observe that it only tests string presence in `canvas_renderer.js` and not in `combat_feel_engine.js`.

3. **Verify Line Counts & Hygiene Gate**:
   ```bash
   python tools/lint/check_code_and_doc_hygiene.py --strict
   ```

---

## Detailed Findings

### [Critical] Finding 1: Facade Camera Shake via Missing `getShakeOffset()` (INTEGRITY VIOLATION)
- **What**: `canvas_renderer.js` calls `window.combatFeelEngine.getShakeOffset?.()`, but `CombatFeelEngine` does not implement `getShakeOffset()`.
- **Where**: `client/webapp/js/engine/combat_feel_engine.js` (missing method) and `client/webapp/js/engine/canvas_renderer.js:276`.
- **Why**: Optional chaining `?.()` prevents a crash, but `feelOffset` is always `undefined`. The camera defaults to legacy isotropic random shake, rendering the entire directional shake matrix calculation in `CombatFeelEngine.update(dt)` completely inactive. The test `test_camera_shake_uses_combat_feel_offset` asserted string presence in `canvas_renderer.js` without checking `combat_feel_engine.js`.
- **Suggestion**: Implement `getShakeOffset() { return { x: this.shakeOffsetX, y: this.shakeOffsetY }; }` in `CombatFeelEngine` and update the test to verify `getShakeOffset` in `combat_feel_engine.js`.

### [Major] Finding 2: Missing `cancelWindup()` in `WeaponSwingRenderer`
- **What**: `combat_skills.js:283` calls `window.weaponSwingRenderer?.cancelWindup()`, which was claimed to cancel pending weapon swings, but the method does not exist.
- **Where**: `client/webapp/js/engine/weapon_swing_renderer.js`.
- **Why**: When a player cancels an attack by dodging, the character transitions to `dodge`, but the weapon swing visual arc/cone continues playing for its full duration.
- **Suggestion**: Implement `cancelWindup()` in `WeaponSwingRenderer` to deactivate any active effect currently within its windup phase.

### [Major] Finding 3: Hit-Stop Engine Contract Decoupled from Game Loop
- **What**: `CombatFeelEngine.isHitStopped(dt)` is never invoked in `canvas_renderer.js`.
- **Where**: `client/webapp/js/engine/canvas_renderer.js` and `client/webapp/js/engine/combat_feel_engine.js:141`.
- **Why**: The game loop does not pause frame advancement on impact. Furthermore, `combatFeelEngine.hitStopTimer` is never decremented during regular loop updates because `update(dt)` does not update `hitStopTimer`.
- **Suggestion**: Connect `isHitStopped(dt)` into `canvas_renderer.js:renderLoop` or harmonize `hitStopTimer` decrement in `CombatFeelEngine.update(dt)`.

---

## Adversarial Challenge Report

### Challenge Summary
**Overall Risk Assessment**: HIGH

### Challenges

#### [Critical] Challenge 1: Phantom Method Camouflage via Optional Chaining
- **Assumption Challenged**: That using `window.combatFeelEngine.getShakeOffset?.()` integrates camera screen shake with `CombatFeelEngine`.
- **Attack Scenario**: Inspecting runtime properties of `window.combatFeelEngine` in browser DevTools or Node.js.
- **Blast Radius**: Directional screen shake along strike vectors is completely inoperative; camera shakes at lower fidelity using legacy random numbers; tests report 100% pass despite broken functionality.
- **Mitigation**: Require runtime execution tests or unit tests that instantiate the class and assert method presence, rather than file string checks.

#### [High] Challenge 2: Lingering Weapon Swing VFX After Dodge Cancel
- **Assumption Challenged**: That attack cancelling via dodge cancels weapon swing visuals.
- **Attack Scenario**: Player triggers heavy slam (`SLAM`), immediately presses Space to dodge within 50ms.
- **Blast Radius**: The player rolls away while the ground slam visual effect spawns at their previous position and completes its 0.54s animation, desynchronizing visual animation from character state.
- **Mitigation**: Implement `cancelWindup()` on `WeaponSwingRenderer`.

---

## Verified Claims

- 5-Phase Kinetic State Machine (`idle`, `run`, `wind_up`, `impact`, `recovery`, `hit_stop`, `dodge`) defined in `animation_engine.js` $\rightarrow$ verified via code inspection and Node.js $\rightarrow$ PASS.
- Pop-in bounce scale ($1.6 \rightarrow 1.0$ normal, $2.0 \rightarrow 1.2$ crit in first 20% of lifetime) $\rightarrow$ verified via math formula analysis in `combat_feel_engine.js` $\rightarrow$ PASS.
- Screen shake clamping to $[2.5, 8.5]$ intensity and $[0.12, 0.45]\text{s}$ duration $\rightarrow$ verified in `combat_feel_engine.js` $\rightarrow$ PASS.
- Dodge roll cancels `isActionLocked` and `attackTimer` $\rightarrow$ verified in `combat_skills.js` $\rightarrow$ PASS.
- Line length hygiene gates ($\le 500$ hard cap) $\rightarrow$ verified via `python tools/lint/check_code_and_doc_hygiene.py --strict` $\rightarrow$ PASS.
- Unit test suite execution $\rightarrow$ `pytest tests/unit/test_m2_animation_and_combat_feel.py -v` (30/30 passed) $\rightarrow$ PASS (with caveat on string assertions).
- E2E test suite execution $\rightarrow$ `pytest tests/e2e/test_poe2_ui_animation_vfx_e2e.py -v` (33 passed, 9 xpassed) $\rightarrow$ PASS.