# MILESTONE 1 REVIEW & ADVERSARIAL CHALLENGE ANALYSIS

**Author**: `reviewer_m1_2` (Reviewer & Adversarial Critic)  
**Parent Agent**: `orchestrator_22` (`34037784-62e1-41f8-bfe6-912696fdec14`)  
**Scope**: Milestone 1 Deliverables (VFX Albedo & Normal Atlases, ASTC 4x4 Container, UV Manifest, Monster & Character Scaffolding, E2E Test Suite)  
**Date**: 2026-10-04T12:58:30Z  

---

## 1. Review Summary

**Verdict**: **REQUEST_CHANGES**

While the core binary VFX generation (`savage_primal_skills_vfx_atlas.png`, `_normal.png`, `.astc`, `.json`) is robust and mathematically sound (achieving 100% pass on 20 E2E tests), the delivery contains **2 Critical Defects** and **1 Major Verification Gap** that prevent production release:
1. **Critical Defect (Scaffolding UV Coordinate Overflow)**: In `monster_character_pipeline_scaffold.py`, the generator sets `textureHeight: 2048`, but advances rows across 48 rows for monsters (7,680 px) and 80 rows for characters (15,360 px). Consequently, **74.1% of monster frames** ($160 / 216$) and **85.7% of character frames** ($480 / 560$) have normalized vertical UV coordinates $V \in [1.0, 7.5]$, completely breaking texture sampling in Cocos Creator and WebGL/Metal shaders.
2. **Critical Defect (Codebase Hygiene Hard Cap Violation)**: The test suite `tests/e2e_cocos/test_vfx_texture_atlas_pipeline_e2e.py` is **575 lines**, violating the strict 500-line hard cap mandated by `GEMINI.md` and failing `tools/lint/check_code_and_doc_hygiene.py --strict`.
3. **Major Defect (Superficial Test Coverage / Self-Certifying Verification)**: Test `test_t1_monster_and_character_scaffolding_structure` only checks whether directory paths exist (`has_scaffold_script or has_resource_dirs`), masking the massive out-of-bounds UV defect.

---

## 2. Findings

### [Critical] Finding 1: Scaffolded Monster and Exile Character Manifests Suffer Massive Out-of-Bounds UV Coordinates ($V \in [1.0, 7.5]$ on a $2048 \times 2048$ Atlas)
- **What**: In `tools/asset_pipeline/monster_character_pipeline_scaffold.py`, both monster and character manifests declare `"textureWidth": 2048, "textureHeight": 2048`. However, the layout algorithm assigns each `(action, direction)` pair to an incremental row without 2D column packing:
  - **Monsters (10 archetypes)**: 6 actions $\times$ 8 directions = 48 rows. At $160\text{ px/row}$, total height required is $7,680\text{ px}$. UVs are calculated as $y / 2048.0$, causing 160 of 216 frames ($74.1\%$) to exceed $V = 1.0$, reaching up to $V = 3.75$.
  - **Exile Characters (6 classes)**: 10 actions $\times$ 8 directions = 80 rows. At $192\text{ px/row}$, total height required is $15,360\text{ px}$. UVs reach up to $V = 7.50$ ($85.7\%$ of frames exceed $1.0$).
- **Where**:
  - `tools/asset_pipeline/monster_character_pipeline_scaffold.py:101-107`, `172-178`
  - `client/cocos/assets/resources/monsters/archetypes/*/*_anim_manifest.json` (all 10 manifests)
  - `client/cocos/assets/resources/characters/*/*_anim_manifest.json` (all 6 manifests)
- **Why**: Texture coordinates $U, V$ in OpenGL, Apple Metal, and Cocos Creator must be normalized within $[0.0, 1.0]$. Sampling with $V > 1.0$ causes texture repeat wrapping or clamp-to-edge distortion, resulting in corrupted or blank sprites for every action after `idle` and `walk`.
- **Suggestion**:
  1. Refactor `monster_character_pipeline_scaffold.py` to either:
     - Expand `textureHeight` to match the true Power-of-Two requirement ($8192$ for monsters, $16384$ for characters) and compute UVs with `y / textureHeight`, OR
     - Implement proper 2D packing across the unused columns (since an 8-column layout only uses $8 \times 160 = 1280\text{ px}$ out of $2048\text{ px}$ width, leaving 768 px unused), OR
     - Segment each archetype into action-specific manifests (e.g. `mob_xxx_movement_manifest.json`, `mob_xxx_combat_manifest.json`).
  2. Regenerate all 10 monster and 6 character manifest files.

### [Critical] Finding 2: Test Suite `test_vfx_texture_atlas_pipeline_e2e.py` Exceeds the 500-Line Code Hard Cap
- **What**: `tests/e2e_cocos/test_vfx_texture_atlas_pipeline_e2e.py` is currently **575 lines**, exceeding the 500-line limit by 75 lines.
- **Where**: `tests/e2e_cocos/test_vfx_texture_atlas_pipeline_e2e.py`
- **Why**: `GEMINI.md` Section 2.8 and `ENGINEERING_STANDARDS_2026.md` enforce: "Code logic $\le 350-500$ dòng. Hard Cap bắt buộc." Running `python tools/lint/check_code_and_doc_hygiene.py --strict` fails with exit code 1:
  ```
  ❌ [CODE] tests\e2e_cocos\test_vfx_texture_atlas_pipeline_e2e.py (575 dòng) -> Vượt quá Hard Cap (575 > 500 dòng). Bắt buộc phân tách module!
  ```
- **Suggestion**: Split the E2E test suite into two co-located, modular files:
  - `tests/e2e_cocos/test_vfx_texture_atlas_pipeline_e2e.py` (Tiers 1 & 2: Feature Coverage & Boundary Checks, ~280 lines)
  - `tests/e2e_cocos/test_vfx_texture_atlas_combinations_e2e.py` (Tiers 3 & 4: Combinations & Workload Scenarios, ~295 lines).

### [Major] Finding 3: Shallow Assertion in `test_t1_monster_and_character_scaffolding_structure`
- **What**: Test 6 (`test_t1_monster_and_character_scaffolding_structure`) only asserts:
  ```python
  assert has_scaffold_script or has_resource_dirs
  ```
  It does not validate the content of the generated manifests, frame counts, direction coverage, or UV boundedness ($[0.0, 1.0]$).
- **Where**: `tests/e2e_cocos/test_vfx_texture_atlas_pipeline_e2e.py:231-250`
- **Why**: This shallow check self-certified the scaffolding deliverable, allowing the critical UV overflow defect in Finding 1 to pass undetected in CI.
- **Suggestion**: Add a dedicated UV boundary assertion in the test suite that inspects all generated monster and character manifests and validates:
  ```python
  assert 0.0 <= u0 <= 1.0 and 0.0 <= v0 <= 1.0 and 0.0 <= u1 <= 1.0 and 0.0 <= v1 <= 1.0
  ```

### [Minor] Finding 4: Fallback Void-Extent ASTC Containers in Dev Environment
- **What**: `tools/asset_pipeline/astc_compressor.py` synthesizes void-extent LDR ASTC containers when `astcenc` is not in PATH. While these containers strictly comply with the Khronos ASTC specification (canonical 16-byte header, 128-bit void-extent LDR blocks encoding average block colors) and allow engines to load the binary format without crashing, they are flat-color blocks rather than full-fidelity compressed textures.
- **Where**: `tools/asset_pipeline/astc_compressor.py:156-184`
- **Why**: Perfectly acceptable for local dev and unit/E2E test pipelines where `astcenc` is not present, but production release pipelines targeting Apple App Store need `astcenc` in the build runner.
- **Suggestion**: Add a note in build documentation confirming that production releases run on build agents with `astcenc` installed.

---

## 3. Verified Claims

| # | Claim | Expected Metric | Measured Result | Verdict |
|---|---|---|---|:---:|
| 1 | Power-of-Two Atlas Dimensions | $2048 \times 2048$ RGBA | $2048 \times 2048$ RGBA (4,194,304 texels) | **PASS** |
| 2 | Alpha Channel Cleanliness & Dilation | 8-iteration dilation, $\ge 50\%$ fringe | Opaque: 103,532 px (2.47%), Fringe dilation: 88.4% | **PASS** |
| 3 | Tangent-Space Normal Map Blue Mean | $\mu_B > 128.0$ (OpenCV BGR layout) | Mean R=127.96, G=128.00, **B=254.69** | **PASS** |
| 4 | Tangent Normal Vector Unit Length | $\|\vec{N}\| = 1.0 \pm 0.08$ | Mean $\|\vec{N}\| = 1.0000$ (Min: 0.9878, Max: 1.0097) | **PASS** |
| 5 | NaN / Inf Free Normal Map | 0 NaNs, 0 Infs | Verified zero NaNs and zero Infs | **PASS** |
| 6 | 12 Active Skills Present in Manifest | Skills 1001 to 1012 | 12 / 12 present (all clip keys and numeric IDs) | **PASS** |
| 7 | 5 Support Sigils Present in Manifest | Sigils 2001, 2002, 2004, 2005, 2006 | 5 / 5 present | **PASS** |
| 8 | Evasion Skill Present in Manifest | Skill 1099 (Huyễn Ảnh Bộ) | Present (`skill_1099`, `ghost_trail_evasion`) | **PASS** |
| 9 | Isometric Grounding Pivot | Anchor $[0.5, 0.90]$ | Exactly $[0.5, 0.90]$ across all frames | **PASS** |
| 10 | Cross-Platform Manifest Parity | Cocos $\leftrightarrow$ WebApp $\leftrightarrow$ Approved | SHA-256 match on all 4 files across 3 directories | **PASS** |
| 11 | ASTC 4x4 Container Header Specs | 16-byte Khronos header, 4x4x1 block | Magic: `0x13aba15c`, Block: 4x4x1, Dim: 2048x2048x1 | **PASS** |
| 12 | ASTC Binary Container File Size | $16 + 262144 \times 16 = 4,194,320$ bytes | Exactly 4,194,320 bytes | **PASS** |
| 13 | 10 Monster Archetypes Scaffolding | 10 genus directories, 8 directions | 10 present, all have 8 directions in manifest | **PASS** |
| 14 | 6 Exile Character Scaffolding | 6 class directories, Song Binh config | 6 present, all have primary + secondary pairs | **PASS** |
| 15 | Cocos TypeScript Compilation | `npm run build:web` (`tsc --noEmit`) | Exit code 0, 0 compilation errors | **PASS** |

---

## 4. Adversarial Challenge & Stress Test Results

### Challenge 1: Normal Map Tangent Vector Inversion Attack
- **Hypothesis**: Does the OpenCV BGR channel inversion bug re-emerge under edge conditions (e.g. transparent border vs. interior texels)?
- **Stress Test**: Sampled 100,000 random pixels across transparent and opaque regions.
- **Result**: Transparent texels map to BGR `[255, 128, 128]` $\implies$ RGB `[128, 128, 255]`, which represents vector $(0, 0, 1)$. Opaque texels have $N_z \in [0.85, 1.0]$. Blue channel mean is $254.69$, never dipping below $128.0$.
- **Outcome**: **ROBUST (Passed)**.

### Challenge 2: UV Coordinate Boundedness Across ALL Deliverables
- **Hypothesis**: Are all frame coordinates across all generated manifests bounded in $[0.0, 1.0]$?
- **Stress Test**: Parsed all 18 manifests on disk (1 VFX manifest + 10 monster manifests + 6 character manifests + 1 NPC manifest) and inspected every frame's UV coordinates.
- **Result**:
  - VFX Manifest (`savage_primal_skills_vfx_atlas.json`): 72 unique frames, all UV $\in [0.0, 1.0]$.
  - Monster Manifests (10 files): 216 frames each. In every monster manifest, **160 frames have $V > 1.0$ (up to $V = 3.75$)**.
  - Character Manifests (6 files): 560 frames each. In every character manifest, **480 frames have $V > 1.0$ (up to $V = 7.50$)**.
- **Outcome**: **FAILURE CONFIRMED (Critical Defect)**.

### Challenge 3: Frame Position Disjointness (Collision Attack)
- **Hypothesis**: Do any frames in `savage_primal_skills_vfx_atlas.json` stomp on each other's pixel coordinates?
- **Stress Test**: Mapped all frame rects $(x, y, w, h)$ into a $16 \times 16$ grid.
- **Result**: Exactly 72 grid cells are occupied (corresponding to 18 sequences $\times$ 4 frames). Zero cell overlap between distinct skills/sigils. Aliases share identical coordinates as expected.
- **Outcome**: **ROBUST (Passed)**.

---

## 5. Coverage Gaps & Unverified Items

- **Asset Rendering for Monsters & Characters**:
  - Only directory and manifest scaffolding exists for monsters and characters (Milestone 1 scope). Actual texture atlas rendering for monsters (8 directions $\times$ 6 actions) and heroes (8 directions $\times$ 10 actions) is scheduled for subsequent milestones.
- **Runtime Performance on Apple Silicon Hardware**:
  - Validated via static analysis and ASTC container parsing on Windows; actual Metal API runtime profiling ($120\text{ FPS}$ ProMotion draw call budget $< 8.33\text{ ms}$) must be verified on physical iOS devices in Milestone 4.

---

## 6. Required Action Items to Achieve Approval

To pass verification and receive `APPROVE`, `worker_m1` (or the implementation agent) must execute:

1. **Fix `tools/asset_pipeline/monster_character_pipeline_scaffold.py`**:
   - Correct the UV calculation and grid geometry so that all frame coordinates are strictly within $[0.0, 1.0]$.
   - Ensure the declared `textureWidth` and `textureHeight` match the layout geometry.
   - Re-run `python tools/asset_pipeline/monster_character_pipeline_scaffold.py` to regenerate all 10 monster manifests and 6 character manifests.
2. **Modularize `tests/e2e_cocos/test_vfx_texture_atlas_pipeline_e2e.py`**:
   - Split into two files to bring line counts under the 500-line Hard Cap (e.g. `test_vfx_texture_atlas_pipeline_e2e.py` for Tiers 1-2, and `test_vfx_texture_atlas_combinations_e2e.py` for Tiers 3-4).
   - Add explicit assertions in the test suite checking that all UVs in monster and character manifests satisfy $0.0 \le u, v \le 1.0$.
3. **Verify Hygiene**:
   - Run `python tools/lint/check_code_and_doc_hygiene.py --strict` and confirm zero Hard Cap violations.