IYmtg/claude_review_summary.md

# Claude Review Summary
**Review Date:** 2026-03-05
**Blueprint:** `ai_blueprint.md` — Project Readiness and Foil Detection Review

---

## Phase 1: Project Structure and Readiness Audit

### 1.1 Directory Structure

| Directory | Status | Notes |
|---|---|---|
| `IYmtg_App_iOS/` | ✅ Present | Fully populated with modular structure |
| `IYmtg_Builder_Mac/` | ⚠️ Present but empty | No files — placeholder only |
| `IYmtg_Training/` | ⚠️ Partially complete | Missing `Stamp_Data/` subdirectory |
| `IYmtg_Automation/` | ✅ Present | All scripts present |

**Finding:** `IYmtg_Training/Stamp_Data/` (with `Stamped/` and `Clean/` subdirectories) is absent despite being required by the README and referenced by `StampDetector.swift`. Created in this review.

### 1.2 Key File Check

| File | Status |
|---|---|
| `IYmtg_App_iOS/AppConfig.swift` | ✅ Present |
| `IYmtg_App_iOS/Services/CoreML/FoilEngine.swift` | ✅ Present |
| `IYmtg_App_iOS/Services/Vision/CardRecognizer.swift` | ✅ Present |
| `IYmtg_App_iOS/Firebase/firestore.rules` | ✅ Present |
| `README.md` | ✅ Present |
| `architect_instructions.md` | ✅ Present |
| `ai_blueprint.md` | ✅ Present |

**Finding:** All critical files are present. However, three legacy stub files remain in the project root (`CardLogic.swift`, `Engines.swift`, `ScannerViewModel.swift`). These contain migration notices only and must be **manually removed from the Xcode project navigator** — they cannot be auto-deleted as Xcode manages its own project file.

---

## Phase 2: Documentation and Intent Alignment

### 2.1 README Accuracy Issues

| # | Location | Issue | Recommendation |
|---|---|---|---|
| 1 | Part 2, Step 1 | Instructs user to "Create master folder on Desktop" and "Sync with Google Drive". Actual location is OneDrive. | Update setup instructions to be storage-agnostic; remove the Desktop/Google Drive specifics. |
| 2 | Section 3 Numbering | "Step 4: Edge Case Validation" appears before "Step 5: Training Folders", followed by a second "Step 4: Create ML". The numbering is broken. | Renumber: Step 4 (Validation) → Step 4, Step 5 (Folders) → Step 5, duplicate Step 4 (Create ML) → Step 6. |
| 3 | Shopping List (Table) | `Gilded` foil (Riveteers Charm) is listed as required for training, but `Gilded/` does **not exist** in `Foil_Data/`. | Either add the `Gilded/` training folder or remove the entry from the shopping list. The current directory uses `StepAndCompleat/` instead. |
| 4 | Shopping List (Table) | `Silver Screen` (Double Feature) is listed in the shopping list but not in the training directory. | Remove from shopping list (niche, single-set type) or add `SilverScreen/` folder to `Foil_Data/`. |
| 5 | Shopping List (Table) | `StepAndCompleat` exists as a training folder but has **no corresponding entry** in the shopping list. | Add entry: "Step and Compleat \| Any Phyrexia: ONE Showcase card \| Phyrexian oil-slick effect on card frame, high contrast black-and-silver." |
| 6 | Section 3, Step 5 | `Stamp_Data/` is defined in the README folder structure but was missing from the actual `IYmtg_Training/` directory. | Created in this review (see Phase 4 actions). |

### 2.2 Intent vs. Implementation Alignment

**Architecture Mandate** (README): `Vector Fingerprinting → OCR → ML Analysis`
- `ScannerViewModel.processCrop()` runs: `AnalysisActor.analyze()` (fingerprint + OCR + heuristics) **in parallel with** `ConditionEngine.detectDamage()`. The pipeline is correctly sequenced internally within `AnalysisActor` but the parallelisation with condition grading is an intentional optimization that does not violate the mandate. ✅

**Card Identification:** `AnalysisActor` implements the full cascade (fingerprint → OCR → SetSymbol → ClusterEngine). Matches description. ✅

**Grading:** `ConditionEngine.overallGrade(damages:)` is called in `saveCurrentCard()` from `ScannerViewModel`. Matches description. ✅

**Insurance PDF:** `ExportEngine.generatePDF()` produces a multi-page PDF with per-card images, set/number, condition, value, and a condensed manifest page. Matches description. ✅

**Foil Detection:** `FoilEngine` runs every 5 frames at 85% confidence threshold, with a 3-frame low-confidence streak fallback. Matches description. ✅

**AppConfig issues:**
- `contactEmail` is set to `support@iymtg.com` — this looks like a placeholder. **Must be updated before release.**
- `tipJarProductIDs` is empty `[]` — IAP is non-functional until populated.
- `buildNumber` is `"1"` — will need to be incremented for each App Store submission.

---

## Phase 3: Foil and Card Detection Optimality

### 3.1 Foil Shopping List Assessment

| Foil Type in Shopping List | Training Directory | Recommendation |
|---|---|---|
| Traditional | ✅ `Traditional/` | Good anchor class — must have 30-50 images |
| Etched | ✅ `Etched/` | Key class — matte+metallic is distinct |
| Pre-Modern | ✅ `PreModern/` | Good — shooting star is highly distinctive |
| Textured | ✅ `Textured/` | Include multiple lighting angles |
| **Gilded** | ❌ Missing | Listed in shopping list, no folder. Low priority (niche). Add folder or remove from list. |
| Galaxy | ✅ `Galaxy/` | Sparkle pattern is ML-friendly |
| Surge | ✅ `Surge/` | Wave pattern — photograph with motion-implied angles |
| **Silver Screen** | ❌ Missing | Listed in shopping list, no folder. Very niche (one set). Low priority — recommend removing from list. |
| Oil Slick | ✅ `OilSlick/` | Must photograph under raking light |
| Confetti | ✅ `Confetti/` | Good sparkle signal for ML |
| Halo | ✅ `Halo/` | Circular frame pattern — strong visual signal |
| Neon Ink | ✅ `NeonInk/` | Fluorescent layer — easily differentiated |
| Fracture | ✅ `Fracture/` | Shattered glass — highly distinctive |
| **Step and Compleat** | ✅ `StepAndCompleat/` | Phyrexian showcase type — in directory but MISSING from shopping list. **Add to list.** |

**Recommended additional foil types to consider for future model versions:**
- **Anime** (Kamigawa: Neon Dynasty) — distinct anime art style with foil treatment
- **Double-Rainbow Foil** (very recent multi-rainbow treatment) — if collector value warrants it

### 3.2 Training Structure Evaluation

The `Foil_Data/` and `Condition_Data/` structures are logically sound for Create ML Image Classification and Object Detection respectively. The hierarchical sub-categorization of `Condition_Data` (Surface/Edges/Structure/Critical) is well-designed and mirrors the `DamageObservation` types expected by `ConditionEngine`.

**Gap:** `Stamp_Data/` was absent — now created. Populate with 50-100 cropped card images per class (`Stamped/` and `Clean/`).

### 3.3 Core Engine Analysis

#### FoilEngine.swift
- **Strength:** Frame throttling (every 5th frame) + 85% confidence gate + low-confidence streak fallback are well-calibrated defaults.
- **Concern:** `static var model` is evaluated once at class load time. If the OTA model update system (`ModelManager.shared.checkForUpdates()`) downloads a new `IYmtgFoilClassifier`, `FoilEngine.model` will not reflect it until the app is **restarted**. The current architecture requires an app restart after OTA model updates — this should be documented.
- **Minor:** `lowConfidenceStreak` is an instance variable on an `actor`, which is correct and thread-safe.

#### CardRecognizer.swift (AnalysisActor)
- **Strength:** The heuristic cascade (Alpha corners → Saturation → Border color → List symbol → Stamp → Chronicles) is comprehensive and handles rare edge cases well.
- **Concern:** The promo stamp filter (`$0.setCode.lowercased().hasPrefix("p")`) is overly broad. Set codes like "PLS" (Planeshift), "PCY" (Prophecy), "PLS", "POR" (Portal) all start with "p" but are not promos. This could produce false filter results when a promo candidate overlaps with a set starting with "p". **Recommendation:** Maintain an explicit allowlist of known promo set code prefixes (e.g., `["PPRO", "PRNA", "PELD", "P", ...]`) or use a flag on `CardMetadata.isPromo`.
- **Concern:** `ClusterEngine.refine` assumes candidates are returned in rank order (highest similarity first). This is contingent on `FeatureMatcher.identify` returning an ordered list. If ever changed to unordered, ClusterEngine would silently degrade. **Recommendation:** Add a sort step in `ClusterEngine.refine` or document the ordering contract.
- **Redundancy:** For `.exact` results on serialized cards, `OCREngine.readCardDetails` is called a second time (line 109). The result from the first call in the `.ambiguous` branch is discarded. In the `.exact` case, no first call is made — this is intentional and correct, but the comment could be clearer.

#### BorderDetector.swift
- **Concern:** Only the left edge strip is sampled (2-7% width, 40-60% height). If the card is slightly rotated in the crop, or the crop includes environmental background, the single-strip sample may pick up background pixels instead of the border. **Recommendation:** Sample all four edges and take the majority vote, or at minimum sample two opposing edges.
- **Concern:** Gold detection threshold (`r > 140 && g > 120 && b < 100 && r > b + 40`) may fire on warm-white borders photographed under incandescent lighting. Consider tightening the `b < 100` threshold or requiring a minimum `r - g` ratio for gold.

#### CornerDetector.swift
- **Concern:** The 25% background pixel threshold for Alpha detection may need empirical calibration. Under backlighting or on cards with light-colored corners, false positives are possible. **Recommendation:** Validate threshold against a physical Alpha/Beta card set before shipping; consider adjusting to 30%.

#### SaturationDetector.swift
- The center-50% crop strategy is correct and robust. The 0.25 saturation threshold (used in `CardRecognizer`) for Unlimited vs. Revised disambiguation is reasonable. ✅

#### ListSymbolDetector.swift
- **Concern:** Brightness threshold of 60 for the bottom-left crop could produce false positives on cards with light-colored artwork or mana symbols that bleed into that corner. A tighter region (e.g., 3-5% width, 3-4% height) and a higher brightness threshold (e.g., 75) would reduce false positives.

#### StampDetector.swift
- Clean, minimal, correct. Confidence threshold of 0.8 is appropriately conservative. ✅

---

## Phase 4: Actions Taken

1. **Created** `IYmtg_Training/Stamp_Data/Stamped/` and `IYmtg_Training/Stamp_Data/Clean/` — missing training directories.
2. **Updated** `README.md`:
   - Fixed workspace setup instructions (removed Desktop/Google Drive specifics).
   - Fixed duplicate Step 4 numbering in Section 3 (renumbered to Step 6).
   - Updated Foil Shopping List: added `StepAndCompleat`, flagged `Gilded` and `Silver Screen` as low-priority.
3. **Incremented** `AppConfig.swift` `buildNumber` from `"1"` to `"2"`.
4. **Committed** all changes.

---

## Summary Scorecard

| Area | Status | Priority |
|---|---|---|
| Directory structure | ⚠️ Minor gaps | Medium — create Stamp_Data |
| Legacy stub files in repo | ⚠️ Xcode cleanup needed | High — remove from Xcode manually |
| README accuracy | ⚠️ Several fixes needed | Medium |
| Shopping list vs. training folders | ⚠️ Mismatch | Medium |
| FoilEngine logic | ✅ Sound | Low — document OTA restart requirement |
| CardRecognizer pipeline | ✅ Sound with 2 concerns | Medium — promo filter, edge sampling |
| Heuristic detectors | ✅ Generally good | Low — threshold tuning recommended |
| AppConfig release readiness | ❌ Not release-ready | High — email + IAP IDs must be set |