PlantGuide/Docs/ARCHITECTURE_REMEDIATION_PLAN.md

# Architecture Remediation Plan

**Project:** PlantGuide iOS App
**Date:** 2026-01-23
**Priority:** Pre-Production Cleanup
**Estimated Scope:** Medium (1-2 sprint cycles)

---

## Overview

This plan addresses architectural concerns identified during code review. Items are prioritized by risk and impact to production stability.

---

## Phase 1: Critical Fixes (Do First)

These items could cause runtime crashes or data inconsistency if not addressed.

### Task 1.1: Fix Repository Protocol Conformance

**File:** `PlantGuide/Data/Repositories/InMemoryPlantRepository.swift`

**Problem:** `InMemoryPlantRepository` is returned as `PlantRepositoryProtocol` in DIContainer but doesn't actually conform to that protocol.

**Action Items:**
- [ ] Add `PlantRepositoryProtocol` conformance to `InMemoryPlantRepository`
- [ ] Implement any missing required methods from the protocol
- [ ] Verify compilation succeeds
- [ ] Add unit test to verify conformance

**Acceptance Criteria:**
- Code compiles without warnings
- `DIContainer.plantRepository` returns a valid `PlantRepositoryProtocol` instance

---

### Task 1.2: Fix Core Data Stack Thread Safety

**File:** `PlantGuide/Data/DataSources/Local/CoreData/CoreDataStack.swift`

**Problem:** `lazy var persistentContainer` initialization isn't thread-safe despite class being marked `@unchecked Sendable`.

**Action Items:**
- [ ] Replace `lazy var` with a thread-safe initialization pattern
- [ ] Option A: Use `DispatchQueue.sync` for synchronized access
- [ ] Option B: Use an actor for the Core Data stack
- [ ] Option C: Initialize in `init()` instead of lazily
- [ ] Add concurrency tests to verify thread safety

**Acceptance Criteria:**
- No race conditions when accessing `persistentContainer` from multiple threads
- Existing Core Data tests still pass

---

### Task 1.3: Replace Unowned with Weak References in DIContainer

**File:** `PlantGuide/Core/DI/DIContainer.swift`

**Problem:** `[unowned self]` in lazy closures could crash if accessed after deallocation.

**Action Items:**
- [ ] Find all `[unowned self]` captures in DIContainer (lines ~121, 150, 196, 204, 213)
- [ ] Replace with `[weak self]` and add guard statements
- [ ] Example pattern:
  ```swift
  LazyService { [weak self] in
      guard let self else { fatalError("DIContainer deallocated unexpectedly") }
      return PlantNetAPIService.configured(...)
  }
  ```
- [ ] Consider if fatalError is appropriate or if optional return is better

**Acceptance Criteria:**
- No `unowned` references in DIContainer
- App behaves correctly during normal lifecycle

---

## Phase 2: Architectural Consistency (High Priority)

These items address design inconsistencies that affect maintainability.

### Task 2.1: Resolve SwiftData vs Core Data Conflict

**Files:**
- `PlantGuide/PlantGuideApp.swift`
- `PlantGuide/Item.swift`

**Problem:** App initializes SwiftData ModelContainer but uses Core Data for persistence. `Item.swift` appears to be unused template code.

**Action Items:**
- [ ] **Decision Required:** Confirm with team - Are we using SwiftData or Core Data?
- [ ] If Core Data (current approach):
  - [ ] Remove `sharedModelContainer` from `PlantGuideApp.swift`
  - [ ] Remove `.modelContainer(sharedModelContainer)` modifier
  - [ ] Delete `Item.swift`
  - [ ] Remove `import SwiftData` if no longer needed
- [ ] If migrating to SwiftData (future):
  - [ ] Document migration plan
  - [ ] Keep current Core Data as transitional

**Acceptance Criteria:**
- Only one persistence technology in active use
- No dead code related to unused persistence layer

---

### Task 2.2: Unify Repository Implementation

**Files:**
- `PlantGuide/Core/DI/DIContainer.swift`
- `PlantGuide/Data/Repositories/InMemoryPlantRepository.swift`

**Problem:** `plantRepository` returns in-memory storage while `plantCollectionRepository` returns Core Data. This creates potential data sync issues.

**Action Items:**
- [ ] **Decision Required:** Should all plant data use Core Data in production?
- [ ] If yes (recommended):
  - [ ] Update `DIContainer.plantRepository` to return `_coreDataPlantStorage.value`
  - [ ] Ensure `CoreDataPlantStorage` conforms to `PlantRepositoryProtocol`
  - [ ] Keep `InMemoryPlantRepository` for testing only
  - [ ] Add `#if DEBUG` around sample data in `InMemoryPlantRepository`
- [ ] Update any code that assumes in-memory behavior
- [ ] Run full test suite to verify no regressions

**Acceptance Criteria:**
- Single source of truth for plant data in production
- Tests can still use in-memory repository via DI

---

### Task 2.3: Guard Sample Data with DEBUG Flag

**File:** `PlantGuide/Data/Repositories/InMemoryPlantRepository.swift`

**Problem:** `seedWithSampleData()` runs in production builds.

**Action Items:**
- [ ] Wrap `seedWithSampleData()` call in `init()` with `#if DEBUG`:
  ```swift
  private init() {
      #if DEBUG
      seedWithSampleData()
      #endif
  }
  ```
- [ ] Consider making sample data opt-in via a parameter
- [ ] Verify production builds don't include sample plants

**Acceptance Criteria:**
- Release builds start with empty repository
- Debug builds can optionally include sample data

---

## Phase 3: Clean Architecture Compliance (Medium Priority)

These items improve separation of concerns and testability.

### Task 3.1: Extract UI Extensions from Domain Enums

**Files:**
- `PlantGuide/Domain/Entities/Enums.swift` (source)
- Create: `PlantGuide/Presentation/Extensions/CareTaskType+UI.swift`
- Create: `PlantGuide/Presentation/Extensions/Enums+UI.swift`

**Problem:** Domain enums import SwiftUI and contain UI-specific code (colors, icons), violating Clean Architecture.

**Action Items:**
- [ ] Create new file `Presentation/Extensions/Enums+UI.swift`
- [ ] Move these extensions from `Enums.swift`:
  - `CareTaskType.iconName`
  - `CareTaskType.iconColor`
  - `CareTaskType.description`
  - `LightRequirement.description`
  - `WateringFrequency.description`
  - `FertilizerFrequency.description`
  - `HumidityLevel.description`
- [ ] Remove `import SwiftUI` from `Enums.swift`
- [ ] Verify all views still compile and display correctly

**Acceptance Criteria:**
- `Domain/Entities/Enums.swift` has no SwiftUI import
- All UI functionality preserved in Presentation layer
- Domain layer has zero UI framework dependencies

---

### Task 3.2: Reduce Singleton Usage (Optional Refactor)

**Files:** Multiple

**Problem:** Heavy singleton usage reduces testability and flexibility.

**Action Items:**
- [ ] **Low Priority** - Document current singletons:
  - `DIContainer.shared`
  - `CoreDataStack.shared`
  - `InMemoryPlantRepository.shared`
  - `FilterPreferencesStorage.shared`
- [ ] For new code, prefer dependency injection over `.shared` access
- [ ] Consider refactoring `FilterPreferencesStorage` to be injected
- [ ] Keep `DIContainer.shared` as acceptable app-level singleton

**Acceptance Criteria:**
- New code uses DI patterns
- Existing singletons documented
- No new singletons added without justification

---

## Phase 4: Code Cleanup (Low Priority)

Minor improvements for code hygiene.

### Task 4.1: Remove Unused Template Files

**File:** `PlantGuide/Item.swift`

**Action Items:**
- [ ] Verify `Item` is not referenced anywhere (search project)
- [ ] Delete `Item.swift`
- [ ] Remove from Xcode project if needed

**Acceptance Criteria:**
- No unused files in project

---

### Task 4.2: Review @unchecked Sendable Usage

**Files:**
- `PlantGuide/Data/DataSources/Remote/NetworkService/NetworkService.swift`
- `PlantGuide/Data/DataSources/Local/CoreData/CoreDataStack.swift`

**Action Items:**
- [ ] Audit each `@unchecked Sendable` usage
- [ ] Document why each is safe OR fix the underlying issue
- [ ] Add code comments explaining thread-safety guarantees

**Acceptance Criteria:**
- Each `@unchecked Sendable` has documented justification
- No hidden thread-safety bugs

---

## Task Checklist Summary

### Must Complete Before Release
- [ ] 1.1 - Fix Repository Protocol Conformance
- [ ] 1.2 - Fix Core Data Stack Thread Safety
- [ ] 1.3 - Replace Unowned References
- [ ] 2.1 - Resolve SwiftData vs Core Data
- [ ] 2.2 - Unify Repository Implementation
- [ ] 2.3 - Guard Sample Data

### Should Complete
- [ ] 3.1 - Extract UI Extensions from Domain
- [ ] 4.1 - Remove Unused Template Files

### Nice to Have
- [ ] 3.2 - Reduce Singleton Usage
- [ ] 4.2 - Review @unchecked Sendable

---

## Decision Log

| Decision | Options | Chosen | Date | Decided By |
|----------|---------|--------|------|------------|
| Persistence technology | SwiftData / Core Data | TBD | | |
| Production repository | Core Data / In-Memory | TBD | | |

---

## Notes for Developer

1. **Start with Phase 1** - These are potential crash sources
2. **Run tests after each task** - Ensure no regressions
3. **Create separate commits** - One commit per task for easy review/revert
4. **Update this doc** - Check off items as completed
5. **Ask questions** - Flag any blockers in standup

---

*Document created by: Project Manager*
*Last updated: 2026-01-23*