# Claude Review Summary
**Review Date:** 2026-03-05 (Revision 2 — Full Compilation Readiness Audit)
**Blueprint:** `ai_blueprint.md` — Full Project Readiness Audit

---

## Executive Summary

The source code is architecturally complete and well-structured. Two **Blocker** issues will prevent the test target from compiling. One **Critical** issue (missing `cards.json` generator) means the scanner cannot function at all at runtime. Four **Major** issues represent deprecation warnings and a Firebase data-leak bug. The app target itself will compile once Firebase imports are confirmed.

---

## 1. Dependency Verification

### Package Manager
No `Package.swift`, `Podfile`, or `Cartfile` exists in the repository. Swift Package Manager dependencies are managed through the Xcode `.xcodeproj` file, which lives on the developer's Mac (not in this repo). Dependency versions cannot be verified from source alone.

**Required SPM packages (must be added in Xcode):**

| Package | Required Modules | Min Version |
|---|---|---|
| `firebase-ios-sdk` | FirebaseCore, FirebaseAuth, FirebaseFirestore, FirebaseStorage | ≥ 10.x |
| SwiftData | Built-in (iOS 17+) | iOS 17 SDK |

**Minimum Deployment Target:** iOS 17 is required by `SwiftData`, `@ModelActor`, and `ModelContainer`. This must be set in Xcode → Build Settings.

---

## 2. File & Resource Integrity

### Missing Runtime Files (excluded by `.gitignore` — must be added before shipping)

| File | Referenced In | Status |
|---|---|---|
| `cards.json` | `ScannerViewModel.swift:96` | ❌ **CRITICAL** — Generator (`IYmtg_Builder_Mac/`) is empty |
| `GoogleService-Info.plist` | `IYmtgApp.swift:9` | ⚠️ Expected missing — handled gracefully with local-mode fallback |
| `IYmtgFoilClassifier.mlmodel` | `FoilEngine.swift:12` | ⚠️ Expected missing — app degrades gracefully (foil = "None") |
| `IYmtgConditionClassifier.mlmodel` | `ConditionEngine.swift:17` | ⚠️ Expected missing — app degrades gracefully (grade = "Ungraded") |
| `IYmtgStampClassifier.mlmodel` | `StampDetector.swift:8` | ⚠️ Expected missing — app degrades gracefully |
| `IYmtgSetClassifier.mlmodel` | `SetSymbolEngine.swift:8` | ⚠️ Expected missing — app degrades gracefully |

### Missing Image Assets (must exist in `Assets.xcassets` inside Xcode project)

| Asset Name | Referenced In | Notes |
|---|---|---|
| `scanner_frame` | `ContentView.swift:114` | Required — scanner overlay frame |
| `logo_header` | `ContentView.swift:147` | Required — header logo |
| `empty_library` | `ContentView.swift:230` | Required — empty state illustration |
| `share_watermark` | `ContentView.swift:334` | Optional — has fallback text if missing |

### Placeholder / Orphan Files

| File | Issue |
|---|---|
| `Data/Persistence/PersistenceActor.swift` | Empty placeholder — must be **removed from Xcode project navigator** (note inside the file confirms this) |
| `Features/CardDetail/` | Directory is empty — `CardDetailView` is defined in `ContentView.swift` instead |

---

## 3. Static Code Analysis

### BLOCKER Issues (prevent compilation)

#### B1 — `IYmtgTests.swift:113` — `ScannerViewModel()` no-arg init does not exist
**Severity:** Blocker | **File:** `IYmtg_App_iOS/IYmtgTests.swift` | **Lines:** 113, 137

`ScannerViewModel` only exposes `init(collectionVM: CollectionViewModel)`. The test file calls `ScannerViewModel()` with no arguments in two test functions (`testViewModelFiltering` and `testPortfolioCalculation`). The Swift compiler will reject this.

```swift
// Current (BROKEN):
let vm = ScannerViewModel()

// Fix:
let colVM = CollectionViewModel()
let vm = ScannerViewModel(collectionVM: colVM)
```

#### B2 — `IYmtgTests.swift:118-148` — Accessing non-existent properties on `ScannerViewModel`
**Severity:** Blocker | **File:** `IYmtg_App_iOS/IYmtgTests.swift` | **Lines:** 118, 121, 126, 137, 143

The test directly assigns/reads `vm.scannedList`, `vm.librarySearchText`, `vm.filteredList`, and `vm.portfolioValue` on a `ScannerViewModel` instance. None of these are forwarded computed properties on `ScannerViewModel` — they belong to `CollectionViewModel`. The compiler will produce unresolved identifier errors.

**Fix:** Obtain these properties from `vm.collectionVM` instead:
```swift
vm.collectionVM.scannedList = [card1, card2]
vm.collectionVM.librarySearchText = "Alpha"
// etc.
```

---

### CRITICAL Issues (app non-functional at runtime)

#### C1 — `cards.json` generator is missing (`IYmtg_Builder_Mac/` is empty)
**Severity:** Critical | **File:** `IYmtg_Builder_Mac/` (directory) | **Line:** N/A

`ScannerViewModel.init` attempts to load `cards.json` from the app bundle. If absent, the app immediately shows a "Database Missing" alert and the scanner is permanently non-functional. The `IYmtg_Builder_Mac/` directory — which should contain the Swift CLI tool that generates this file — is completely empty. This is the single most important missing piece before any developer can test the scanner.

---

### MAJOR Issues (warnings or significant functional gaps)

#### M1 — Deprecated `.onChange(of:)` API (iOS 17 deprecation warnings)
**Severity:** Major | **File:** `IYmtg_App_iOS/ContentView.swift` | **Lines:** 172, 237

The single-parameter closure form of `.onChange(of:)` was deprecated in iOS 17. Since the app targets iOS 17+, Xcode will emit deprecation warnings on every build.

```swift
// Line 172 — DEPRECATED:
.onChange(of: scenePhase) { newPhase in ... }

// Fix:
.onChange(of: scenePhase) { _, newPhase in ... }

// Line 237 — DEPRECATED:
.onChange(of: vm.selectedCurrency) { _ in vm.refreshPrices(force: true) }

// Fix:
.onChange(of: vm.selectedCurrency) { _, _ in vm.refreshPrices(force: true) }
```

#### M2 — `ModelManager.swift` missing `import FirebaseCore`
**Severity:** Major | **File:** `IYmtg_App_iOS/Services/CoreML/ModelManager.swift` | **Lines:** 1–3, 35

`ModelManager.swift` uses `FirebaseApp.app()` (a `FirebaseCore` type) but only imports `FirebaseStorage`. In Swift, each file must explicitly import the module it uses. This may compile in some Xcode/SPM configurations where `FirebaseStorage` transitively links `FirebaseCore`, but it is not guaranteed and should be made explicit.

```swift
// Add to ModelManager.swift imports:
import FirebaseCore
```

#### M3 — `CloudEngine.delete(card:)` never called — Firebase data leak on card deletion
**Severity:** Major | **File:** `IYmtg_App_iOS/Features/Collection/CollectionViewModel.swift` | **Line:** 271–278

`CollectionViewModel.deleteCard(_:)` removes the card from SwiftData (via `saveCollectionAsync()`) and deletes the local image, but never calls `CloudEngine.delete(card:)`. Any cards that were previously backed up to Firebase Firestore will remain in the user's Firestore inventory forever after deletion. The method to fix this already exists — it just needs to be called.

```swift
// In CollectionViewModel.deleteCard(_:), after removing from scannedList:
Task { await CloudEngine.delete(card: card) }
```

#### M4 — `CloudEngine.batchUpdatePrices()` is dead code
**Severity:** Major | **File:** `IYmtg_App_iOS/Services/Cloud/CloudEngine.swift` | **Lines:** 49–69

`CloudEngine.batchUpdatePrices(cards:)` is fully implemented but never called anywhere in the codebase. Price updates are reflected in SwiftData + CloudKit automatically, making this function either redundant or an incomplete feature. It should either be wired into the price-refresh flow or removed.

---

### MINOR Issues (low impact, housekeeping)

#### m1 — `Features/CardDetail/` directory is empty
**Severity:** Minor | **File:** `IYmtg_App_iOS/Features/CardDetail/` (directory)

`CardDetailView`, `DashboardView`, `ScannerView`, `WelcomeView`, and `FeatureRow` are all defined in `ContentView.swift`, making it a 469-line monolith. The `CardDetail/` directory exists but holds no files. This violates the project's modular structure intent but does not affect compilation.

#### m2 — `PersistenceActor.swift` placeholder in Xcode project
**Severity:** Minor | **File:** `IYmtg_App_iOS/Data/Persistence/PersistenceActor.swift`

This file contains only a migration comment and no executable code. It must be manually removed from the Xcode project navigator to avoid confusion (the file itself documents this).

#### m3 — `AppConfig.swift` not configured for production
**Severity:** Minor | **File:** `IYmtg_App_iOS/AppConfig.swift` | **Lines:** 24, 27

| Setting | Current Value | Required |
|---|---|---|
| `contactEmail` | `"support@iymtg.com"` | Real developer email (Scryfall policy) |
| `tipJarProductIDs` | `[]` | Real App Store Connect product IDs |
| `buildNumber` | `"2"` | Increment with each App Store submission |

The `validate()` function checks for `"yourdomain.com"` but not for `"iymtg.com"`, so the fatalError guard will not fire in DEBUG with the current placeholder.

#### m4 — OTA model updates require app restart (undocumented)
**Severity:** Minor | **File:** `IYmtg_App_iOS/Services/CoreML/ModelManager.swift` | **Line:** 16

`FoilEngine.model`, `ConditionEngine.model`, `StampDetector.model`, and `SetSymbolEngine.model` are all `static var` properties evaluated once at class-load time. When `ModelManager.checkForUpdates()` downloads a new `.mlmodelc` to Documents, the running app will not use it until restarted. This is expected behavior but should be documented in the README or in-app release notes.

---

## 4. Feature Completeness Review

| Feature | Status | Notes |
|---|---|---|
| Card Scanner (camera, frame capture) | ✅ Complete | `ScannerViewModel` — full pipeline |
| Card Identification (fingerprint + OCR + heuristics) | ✅ Complete | `AnalysisActor`, `FeatureMatcher`, `OCREngine`, heuristics |
| Foil Detection | ✅ Complete | `FoilEngine` — requires trained model |
| Condition Grading | ✅ Complete | `ConditionEngine` — requires trained model |
| Stamp Detection | ✅ Complete | `StampDetector` — requires trained model |
| Set Symbol Detection | ✅ Complete | `SetSymbolEngine` — requires trained model |
| Collection Management | ✅ Complete | `CollectionViewModel` |
| SwiftData + CloudKit Persistence | ✅ Complete | `PersistenceController`, `BackgroundPersistenceActor` |
| JSON → SwiftData Migration | ✅ Complete | `migrateFromJSONIfNeeded()` |
| Scryfall Price Refresh | ✅ Complete | `ScryfallAPI.updateTrends()` |
| PDF Insurance Export | ✅ Complete | `ExportEngine.generatePDF()` |
| CSV / Arena / MTGO Export | ✅ Complete | `ExportEngine` |
| Firebase Backup (manual) | ✅ Complete | `CloudEngine` + `CollectionViewModel.backupAllToFirebase()` |
| OTA Model Updates | ✅ Complete | `ModelManager.checkForUpdates()` |
| Training Image Upload | ✅ Complete | `TrainingUploader` |
| In-App Purchase (Tip Jar) | ✅ Complete | `StoreEngine` — IAP IDs not configured |
| App Review Prompt | ✅ Complete | `ReviewEngine` |
| Training Guide (in-app) | ✅ Complete | `TrainingGuideView` |
| Welcome Screen | ✅ Complete | `WelcomeView` |
| Dashboard (Portfolio stats) | ✅ Complete | `DashboardView` |
| **IYmtg_Builder_Mac (cards.json generator)** | ❌ **MISSING** | Directory exists but is completely empty |
| Unit Tests | ⚠️ Present but broken | 2 Blocker compilation errors (see B1, B2) |

---

## 5. Summary Scorecard

| Category | Status | Priority |
|---|---|---|
| Test target compilation | ❌ 2 Blockers | **Blocker** — fix before any CI |
| Runtime: `cards.json` missing | ❌ Scanner non-functional | **Critical** |
| Deprecated API warnings | ⚠️ 2 occurrences | **Major** |
| Firebase import missing | ⚠️ Potential compile error | **Major** |
| Firebase delete data leak | ⚠️ Silent bug | **Major** |
| Dead code (`batchUpdatePrices`) | ⚠️ Minor bloat | **Major** |
| `CardDetail/` directory empty | ℹ️ Structural inconsistency | Minor |
| `PersistenceActor.swift` placeholder | ℹ️ Xcode cleanup needed | Minor |
| AppConfig production values | ℹ️ Not release-ready | Minor |
| OTA restart undocumented | ℹ️ Developer note needed | Minor |
| SPM deps (no manifest in repo) | ℹ️ Expected (Xcode-managed) | Informational |
| App source code (all other files) | ✅ Compiles, well-structured | — |