Opus 4.6 Swift Challenge Results

# Challenge Results

## Model
Claude Opus 4.6 (`claude-opus-4-6`)

## Timing
- Start: 10:27 AM PST
- Complete: 10:37 AM PST
- Wall clock: **10 minutes**

## Part 1: Architecture

### 1A. Data Flow

Complete path of a single camera frame from hardware capture to action execution:

**1. Hardware -> GCD Queue (CameraManager, sessionQueue)**

`AVCaptureSession` delivers a `CMSampleBuffer` to `FrameCaptureDelegate.captureOutput(_:didOutput:from:)` on the serial `sessionQueue` (`CameraManager.swift:462-491`). The delegate:
- Applies timestamp-based FPS throttling (compares `CMTimeGetSeconds` against `minFrameInterval`)
- Extracts `CVPixelBuffer` via `CMSampleBufferGetImageBuffer`
- Reads `connection.videoRotationAngle` and `connection.isVideoMirrored`, converts to `CGImagePropertyOrientation` via `cgImageOrientation(fromVideoRotationAngle:mirrored:)` (`CameraManager.swift:98-115`)
- Converts `CMTime` to `UInt64` nanoseconds via the `CMTime.nanoseconds` extension (`CameraManager.swift:65-88`)
- Constructs `CapturedFrame` (an `@unchecked Sendable` struct with `nonisolated(unsafe)` CVPixelBuffer storage)

**2. GCD -> Async/Await (AsyncStream boundary)**

`CapturedFrame` is yielded to an `AsyncStream<CapturedFrame>.Continuation` with `.bufferingNewest(1)` policy (`CameraManager.swift:230`). This is the GCD-to-structured-concurrency boundary. If the consumer is behind, only the most recent frame is retained.

**3. Detached Task -> Actor (HandPoseDetector)**

`DetectionService.startDetectionLoop` (`DetectionService.swift:118-145`) spawns a `Task.detached(priority: .userInitiated)` that iterates the frame stream. For each frame, it calls `await detector.detect(frame:)`, hopping into the `HandPoseDetector` actor.

`HandPoseDetector.detect(frame:)` (`HandPoseDetector.swift:91-132`):
- Creates `VNImageRequestHandler(cvPixelBuffer:, orientation:)` from the frame
- Performs `VNDetectHumanHandPoseRequest` synchronously inside `autoreleasepool`
- Extracts `[VNHumanHandPoseObservation]` -> `[HandPose]` via joint-by-joint mapping (`visionToJoint` dictionary), filtering by `minimumJointConfidence`
- Sorts hands by chirality (left < right < unknown), then confidence descending, then wrist.x ascending
- Returns `HandDetectionResult` -- a `Sendable` struct containing `[HandPose]`, `frameTimestamp`, and `processingTime`

**4. Actor -> MainActor (DetectionService state update)**

The detached task calls `await self?.updateWithResult(result)` (`DetectionService.swift:132`), hopping to MainActor since `DetectionService` is `@MainActor`. `updateWithResult` stores the result in `lastResult` and updates diagnostic counters.

**5. MainActor polling loop (AppState classification)**

`AppState.startClassificationLoop()` (`AppState.swift:88-119`) runs a `@MainActor` Task that polls every 100ms. When it detects a new frame (by comparing `frameTimestamp` to `lastClassifiedFrame`), and a session is active (`sessionManager.isMonitoring`), and the result has a `primaryUsableHand`:

**6. MainActor -> Actor (GestureRecognitionService)**

Calls `await recognitionService.classify(pose)` (`AppState.swift:102`), hopping into the `GestureRecognitionService` actor. Inside:
- `HandPose` -> `NormalizedPose`: translates to wrist origin, scales by palm width (index MCP to little MCP distance), mirrors left hands to right-hand space, rotation-canonicalizes around wrist->middleMCP axis (`NormalizedPose.swift:12-63`)
- `GestureClassifier.classify(pose)` runs weighted kNN against stored samples, returns `GestureMatch?` with score, distance, voteShare, distanceMargin (`GestureClassifier.swift:78-103`)
- `GestureStabilizer.update(with:at:)` debounces: pending -> began -> held, with hysteresis for brief dropouts (`GestureStabilizer.swift:81-111`)
- Returns `GestureStabilizerOutput` (.none / .began / .held / .hysteresisHeld)

**7. MainActor -> Actor (ActionDispatchService)**

Back on MainActor, if `output.shouldFireAction` (i.e., `.began`), the `GestureID` is yielded to `gestureBeganContinuation` -- an `AsyncStream<GestureID>` with `.bufferingNewest(32)` (`AppState.swift:104-105`).

`startActionDispatchLoop()` consumes this stream and calls `await actionDispatchService.handleGestureBegan(gestureID:)` (`AppState.swift:80-85`), entering the `ActionDispatchService` actor. This:
- Looks up the `Action` via `GestureActionMap` (e.g., palm -> playPause)
- Enforces per-gesture cooldown (1s default, 30s for permission-denied backoff)
- Calls `executor.execute(action)` on `LiveActionExecutor`

**8. Action -> OS (LiveActionExecutor)**

`LiveActionExecutor.execute` (`LiveActionExecutor.swift:17-31`) dispatches to macOS APIs:
- `.mediaControl` -> `MediaKeyPoster.post()` (synthesizes NX media key CGEvents)
- `.keyboardShortcut` -> `CGEvent` key down/up posted to `.cghidEventTap` (requires `AXIsProcessTrusted()`)
- `.openApplication` -> hops to `@MainActor`, calls `NSWorkspace.shared.openApplication(at:configuration:)`
- `.runShortcut` -> spawns `/usr/bin/shortcuts run <name>` via `Process`, awaits termination via `CheckedContinuation`

**Threading model summary:**

| Stage | Threading | Why |
|-------|-----------|-----|
| Camera capture + delegate | GCD serial queue (`sessionQueue`) | AVFoundation requirement |
| Frame transfer | AsyncStream (GCD -> async boundary) | Backpressure via `.bufferingNewest(1)` |
| Vision processing | `Task.detached` -> `HandPoseDetector` actor | Serial processing, no CPU pileup |
| Detection state update | MainActor (DetectionService) | SwiftUI observation |
| Classification polling | MainActor (AppState) | Reads @Observable state |
| Gesture classification | `GestureRecognitionService` actor | Mutable classifier/stabilizer state |
| Action dispatch | `ActionDispatchService` actor | Mutable cooldown state |
| OS action execution | Varies (CGEvent, MainActor for NSWorkspace, Process) | Framework requirements |

### 1B. Concurrency Architecture

The codebase uses three distinct concurrency strategies:

**1. GCD Serial Queue (CameraManager)**

`CameraManager` wraps `AVCaptureSession` with a dedicated `DispatchQueue(label: "com.app.camera.session", qos: .userInitiated)` (`CameraManager.swift:160`). All session configuration, start/stop, and delegate callbacks run on this queue. The class is `@unchecked Sendable` and uses `NSLock` to protect individual properties (`state`, `targetFPS`) that are read from other contexts.

*Why:* AVFoundation predates Swift concurrency and mandates GCD-based dispatch. `AVCaptureSession` is not Sendable and its operations must serialize on a specific queue. There's no way to wrap this in an actor without fighting the framework.

**2. Swift Actors (HandPoseDetector, GestureRecognitionService, ActionDispatchService)**

These three types use actor isolation to enforce serial access to mutable state:
- `HandPoseDetector` (`actor`) -- serializes Vision requests, preventing CPU pileup from concurrent `VNImageRequestHandler.perform` calls
- `GestureRecognitionService` (`actor`) -- protects mutable `classifier`, `stabilizer`, `userLibrary`, and debounced save state
- `ActionDispatchService` (`actor`) -- serializes cooldown tracking (`nextAllowedAtByGesture` dictionary)

*Why:* These types own mutable state that requires serial access and sit naturally in the async/await pipeline. Actors provide compiler-enforced isolation without manual locking.

**3. MainActor Isolation (DetectionService, SessionManager, AppState)**

The UI-facing coordination layer uses `@MainActor @Observable`:
- `DetectionService` -- bridges camera pipeline to observable state (`lastResult`, `state`)
- `SessionManager` -- session lifecycle state machine, observed by menu bar UI
- `AppState` -- owns services, runs classification and dispatch loops

*Why:* SwiftUI observation requires main thread access. These types are the "view model" layer -- they exist to be observed by the UI.

**How data crosses between them safely:**

- **GCD -> async/await:** `AsyncStream<CapturedFrame>` with `.bufferingNewest(1)`. `CapturedFrame` is `@unchecked Sendable` wrapping CVPixelBuffer with `nonisolated(unsafe)` -- safe because single-ownership semantics are enforced by the stream's buffering policy and consumption pattern.
- **async/await -> GCD:** `withCheckedThrowingContinuation` / `withCheckedContinuation` in `CameraManager.start()` and `stop()`. The continuation is resumed from `sessionQueue.async` blocks.
- **Actor <-> MainActor:** Standard `await` calls. All intermediate types (`HandDetectionResult`, `HandPose`, `NormalizedPose`, `GestureMatch`, `GestureStabilizerOutput`, `GestureID`, `Action`) are `Sendable` value types (structs/enums) that cross boundaries freely.
- **MainActor -> unstructured Task -> Actor:** `AppState` uses unstructured `Task` instances for the classification loop (`@MainActor` Task) and action dispatch loop (inherits no isolation, consumes an AsyncStream and hops into `ActionDispatchService` actor).

### 1C. Riskiest Boundary

**The `CVPixelBuffer` crossing from GCD to the detached Task via AsyncStream.**

**What crosses:** `CVPixelBuffer` is not `Sendable`. It's wrapped in `CapturedFrame: @unchecked Sendable` with `nonisolated(unsafe)` storage (`CameraManager.swift:19-24`). The frame is created in `FrameCaptureDelegate.captureOutput` (on sessionQueue), yielded to an `AsyncStream`, and consumed by the detached task in `DetectionService.startDetectionLoop`.

**Why it's risky:**

1. `@unchecked Sendable` entirely bypasses the compiler's thread safety analysis. The safety argument is informal: single-ownership semantics -- frame is created once, yielded once, consumed once. But this contract is enforced by convention and the `.bufferingNewest(1)` policy, not by the type system.

2. `CVPixelBuffer` comes from a `CMSampleBuffer` delivered by AVFoundation. The pixel buffer retains the backing memory via ARC/IOSurface semantics, so it's valid after the delegate callback returns. But this is a subtle invariant -- someone unfamiliar with AVFoundation's memory model might not realize the buffer survives the callback scope.

3. If the buffering policy were changed (e.g., to `.unbounded`), or a second consumer were added to the stream, or the frame were captured in a log statement, the single-ownership guarantee would break silently with no compiler diagnostic.

**Mitigations in place:**
- `.bufferingNewest(1)` limits concurrent outstanding frames to approximately 2 (one being processed, one in the buffer)
- `CapturedFrame` is a value type with `let` fields -- immutable after construction
- Detailed comments document the safety contract (`CameraManager.swift:20-23`)
- The consumption pattern is intentionally linear: one producer (delegate), one consumer (detached task)

**Assessment:** The mitigations are sufficient for the current design. The real risk is future maintainability -- `@unchecked Sendable` is a load-bearing assertion that can't be validated by the compiler. Any modification to the frame pipeline (adding logging, changing buffering, adding a preview consumer) should be accompanied by re-auditing this safety contract. This is about as well as you can do given that `CVPixelBuffer` genuinely can't be made `Sendable`.

**Honorable mention:** The `CameraManager` property access pattern is a secondary risk. `framesContinuation` is written by `makeFrameStream()` (called from MainActor) and read by `addVideoOutput()` / `failSession()` / `stop()` (on sessionQueue) and `deinit` (arbitrary thread). The safety relies on call ordering and GCD memory barrier semantics, not on locks or actor isolation. See CameraManager review item #1 below.

### 1D. State Machine Correctness

**Scenario 1: `startSession` called while already in `.starting`**

```swift
// SessionManager.swift:87
guard case .idle = state else { return false }
```

State is `.starting` -> guard fails immediately -> returns `false`. No side effects, no state corruption. **Correct.**

**Scenario 2: Detection service fails during `startSession` (after `acquire` but before the guard)**

`startSession` calls `await detectionController.acquire(for: .session)` at line 93. Inside `acquire`, if `start()` fails, DetectionService sets its state to `.failed(message)` and returns. Importantly, `handleFailure` / `onFailure` are NOT invoked for start-time failures -- only for mid-session failures (stream ending unexpectedly). So the `onFailure` callback doesn't fire here.

Control returns to `startSession`, which hits the guard at line 95:

```swift
guard detectionController.state == .running,   // .failed -- fails
      case .starting(let pending) = state,
      pending.id == session.id else {
    if case .failed(let message) = detectionController.state {
        lastError = message                    // captures error for UI
    }
    await detectionController.release(for: .session)  // releases lease
    if case .starting = state { state = .idle }        // resets to idle
    return false
}
```

Error is captured, lease is released, state resets to `.idle`, returns `false`. **Correct.**

**Scenario 3: `stopSession` called from `onFailure` callback while `startSession` is in progress**

The `onFailure` callback (`SessionManager.swift:54-59`) creates a new `Task { @MainActor in ... }`. This Task is enqueued on the MainActor run loop but cannot preempt `startSession`, which is also `@MainActor`.

Two sub-scenarios:

**3a: Failure before `startSession` reaches `.running`** -- `startSession` is awaiting `acquire` at line 93. If the detection service fails during this await, the `onFailure` Task fires. But it checks `guard let self, self.isMonitoring else { return }`. `isMonitoring` requires state `.running`, but state is `.starting`, so the guard fails. The callback is a no-op. `startSession` resumes and handles the failure itself via the guard at line 95. **Correct.**

**3b: Failure after `startSession` reaches `.running`** -- `startSession` set state to `.running(session)` at line 107, called `saveState()` and `scheduleExpiration()` (both synchronous, lines 108-109), and returned `true`. Then the camera fails, `onFailure` Task fires, `isMonitoring` is now true, so it calls `stopSession(reason: .error(message))`. `stopSession` sets state to `.stopping`, releases the lease, clears persisted state, and sets state to `.idle`. This is normal mid-session failure handling. **Correct.**

**Edge case worth noting:** If `stopSession` is called directly (not via `onFailure`) while `startSession` is awaiting `acquire` -- e.g., user hits stop before start completes -- then `stopSession` sees state `.starting`, sets state to `.stopping`, and calls `release(for: .session)`. When `acquire` returns and `startSession` resumes, it checks `case .starting(let pending) = state` -- state is now `.stopping` or `.idle`, not `.starting`. The guard fails, and `startSession` calls `release(for: .session)` again. This is a **double release** of the `.session` lease. However, `Set.remove` is idempotent (removing an already-removed element is a no-op), and the subsequent `owners.isEmpty` + `state == .running` check in `DetectionService.release` prevents a double-stop. **Correct, but the double-release is a code smell** -- the state machine handles it safely only because `Set.remove` is inherently idempotent.

---

## Part 2: Code Review

### CameraManager.swift

| # | Severity | Category | Description |
|---|----------|----------|-------------|
| 1 | HIGH | Thread Safety | `makeFrameStream()` writes `framesContinuation` (line 231) from the caller's context (MainActor), but `addVideoOutput()` (line 386), `failSession()` (line 219), `stop()` (line 301), and `deinit` (line 197) read/write it from sessionQueue or arbitrary threads. Safety relies on call ordering (makeFrameStream before start) and GCD's implicit memory barrier on `dispatch_async`, not on any lock or isolation. If `makeFrameStream()` were ever called concurrently with `start()`, or from a context other than its current single-caller pattern, this would be a data race. |
| 2 | MEDIUM | Thread Safety | `deinit` (lines 196-199) accesses `framesContinuation` and `notificationObservers` without dispatching to `sessionQueue`. In practice this is safe because ARC guarantees deinit runs only after all strong references (including sessionQueue blocks that capture `[self]`) are released. But it's a fragile invariant -- if a future change introduced a reference cycle or moved deinit timing, this could race. |
| 3 | MEDIUM | Thread Safety | `FrameCaptureDelegate.targetFPS` (line 444) is `var` with a `didSet` that writes `minFrameInterval` (line 447-448). `CameraManager.targetFPS.set` (line 178-179) writes it from any thread, while `captureOutput` reads `minFrameInterval` on sessionQueue (line 471). This is a technical data race per Swift's memory model, though the consequence is benign (momentarily wrong throttle interval). |
| 4 | MEDIUM | Resource Management | `failSession` (lines 214-221) cleans up `framesContinuation`, `frameDelegate`, and observers, but doesn't remove inputs/outputs from the `captureSession`. The next `start()` -> `setupCaptureSession()` does clear them (lines 343-344), so this is handled. But if `failSession` is the final operation before deallocation, the capture session carries stale configuration into its own `deinit`. AVCaptureSession handles this, but it's an asymmetry with `stop()` which also doesn't remove inputs/outputs. |
| 5 | LOW | Ordering | `setupNotificationObservers()` is called AFTER `captureSession.startRunning()` succeeds (line 272). There's a window between start and observer registration where a runtime error or device disconnect notification would be missed. In practice this window is sub-millisecond and real-world camera failures take time to manifest, but registering observers before `startRunning()` would be strictly more correct. |
| 6 | LOW | Style | `failSession` compresses multiple statements onto single lines with semicolons (lines 217-220). This hinders readability and makes diffing harder. |
| 7 | LOW | Style | `setupNotificationObservers` uses a compact multi-line initialization pattern (lines 203-211) that's clever but not immediately scannable. |

### DetectionService.swift

| # | Severity | Category | Description |
|---|----------|----------|-------------|
| 1 | HIGH | Race Condition | In `acquire(for:)` (lines 62-70): after `await start()`, the method checks `if owners.isEmpty && state == .running { await stop() }` to handle release-during-start. However, `start()` contains multiple `await` points (permission request, camera start). During any suspension, another caller could call `acquire(for: .calibration)`, adding to `owners`, then call `release(for: .calibration)`. If the release happens while `start()` is still awaiting, `owners` transitions from `{.session, .calibration}` -> `{.session}`. When `start()` completes and `acquire` checks `owners.isEmpty`, it's false (`.session` still present). This is fine. But the reverse: if `release(for: .session)` is called during `start()`, owners becomes `{}`. When `start()` returns, `owners.isEmpty` is true, so it stops. But then `startSession` in SessionManager still holds the lease mentally -- it never got a chance to see the running state. The guard in `startSession` (line 95-97) handles this because `detectionController.state` won't be `.running`, so it releases and returns false. **Subtle but handled correctly.** Revised to MEDIUM -- the code is correct but the interleaving is non-obvious and deserves a comment. |
| 2 | MEDIUM | Lifecycle | The `detached Task` in `startDetectionLoop` (line 125) captures `detector` and `cameraManager` strongly. After `stop()` cancels the task (line 181), the task may still be mid-`await detector.detect(frame:)`. It will finish the current Vision request before observing cancellation. During this window, `detector` and `cameraManager` are kept alive by the task even though `stop()` has logically released them. Not a leak (the task will exit), but it delays resource release. |
| 3 | MEDIUM | Error Handling | `onFailure` callback (line 49) is `var` with no access control beyond being on `@MainActor`. It's set once in `SessionManager.init` but could be overwritten by any MainActor code. If overwritten or set to nil, mid-session failures would be silently swallowed. Consider making it `private(set)` or using a delegate protocol. |
| 4 | MEDIUM | Cleanup | When `start()` fails after calling `cameraManager.makeFrameStream()` (line 99) but before `startDetectionLoop` -- e.g., camera permission denied at line 92 or camera start throws at line 102 -- the `AsyncStream` created by `makeFrameStream` is captured in the local `frameStream` variable but never consumed. This is harmless (the stream and its continuation are cleaned up by ARC), but the continuation stored in `cameraManager.framesContinuation` is orphaned until the next `makeFrameStream()` call overwrites it. A defensive `cameraManager.framesContinuation?.finish()` on the failure path would be cleaner. |
| 5 | LOW | Correctness | `updateWithResult` (line 147) is called from the detached task via `await self?.updateWithResult(result)`. Between when the result is produced and when the MainActor hop completes, additional frames may have been processed. The `guard state == .running` check (line 148) prevents updates after stop, but stale results can briefly be assigned to `lastResult` if the pipeline is backed up. In practice, `.bufferingNewest(1)` limits this to at most one stale frame. |
| 6 | LOW | Style | `State.isFailed` and `State.statusText` (lines 212-227) are in an extension at the bottom of the file. These are closely tied to the `State` enum and could live inside the type definition for discoverability. |

### SessionManager.swift

| # | Severity | Category | Description |
|---|----------|----------|-------------|
| 1 | HIGH | Race Condition | `stopSession` can be called while `startSession` is suspended at `await detectionController.acquire(for: .session)` (line 93). When `stopSession` completes (setting state to `.idle`), and `startSession` resumes, the guard at line 95 fails because state is no longer `.starting`. The failure branch calls `await detectionController.release(for: .session)` (line 102) -- but `stopSession` already called `release(for: .session)` (line 125). This is a **double release** of the same lease. It's safe today because `Set.remove` is idempotent and `DetectionService.release` checks state before stopping. But the code doesn't acknowledge this intentional double-release, and a future refactor of `DetectionService.release` that adds side effects to `remove` could introduce bugs. Should either guard against double-release or document the invariant. |
| 2 | MEDIUM | Correctness | `scheduleExpiration` (lines 142-163) polls every 1 second with `Task.sleep(for: .seconds(1))`. For a session with `.timed(5)` duration, this could overshoot by up to 999ms. A more precise approach would be to sleep for exactly `remainingTime`, or use a shorter interval near expiry. The current approach is simpler and handles system sleep/clock adjustments gracefully, but the 1-second granularity may feel imprecise to users watching a countdown timer. |
| 3 | MEDIUM | Resilience | `restoreState()` (lines 180-211) trusts persisted session data to be valid beyond just JSON decodability. If the persisted `Session.Duration` is `.timed(seconds)` with a very large `seconds` value, or `.until(date)` with a far-future date, the session would restore and run effectively forever. There's no upper bound validation. Not a security issue, but could surprise users if UserDefaults data is corrupted. |
| 4 | MEDIUM | Callback Safety | The `onFailure` callback setup in `init` (lines 54-60) creates a new `Task` each time an error occurs. If the detection service fails rapidly (e.g., flaky camera), multiple Tasks are created. Each checks `self.isMonitoring`, so only the first one (while still `.running`) triggers `stopSession`; the rest see `.idle`/`.stopping` and bail. This is correct but wasteful -- a flag or debounce would be cleaner. |
| 5 | LOW | Testability | `persistenceKey` is a hardcoded string (`"com.app.session.active"`, line 48) with `UserDefaults.standard` used directly. This makes it harder to test persistence in isolation -- tests would need to clean up the shared UserDefaults. Injecting the UserDefaults instance and key would improve testability. |
| 6 | LOW | API Design | `startSession` returns `Bool` (line 86) for success/failure, but the error details are in `lastError`. Callers must check two things. A `Result<Session, Error>` return would be more ergonomic and self-documenting. |

---

## Summary

**What's done well:**

- **Clear concurrency boundaries.** The three-strategy approach (GCD for AVFoundation, actors for mutable state, MainActor for UI) is well-chosen and each layer has a clear reason for its isolation model. Data types that cross boundaries are all Sendable value types.
- **Defensive state checks.** Both `DetectionService` and `SessionManager` guard state transitions at every re-entry point after suspension. The `startSession` guard pattern (check state, check detection state, check session ID) catches interleaving scenarios that would be easy to miss.
- **Lease model for camera ownership.** The `acquire`/`release` pattern in `DetectionService` cleanly handles the multi-consumer problem (session vs. calibration) without exposing start/stop to callers who shouldn't manage the lifecycle.
- **Thoughtful value type design.** `CapturedFrame`, `HandPose`, `NormalizedPose`, `GestureMatch`, `GestureStabilizerOutput`, `HandDetectionResult` -- all Sendable structs/enums. No unnecessary reference types in the data pipeline.
- **GestureStabilizer is well-designed.** Time-based (not frame-count-based) stabilization with hysteresis is a solid approach. The edge vs. level distinction (began/held) maps cleanly to action dispatch vs. UI display.

**What needs attention:**

- **CameraManager thread safety is convention-based, not compiler-enforced.** The `@unchecked Sendable` class uses a mix of NSLock, sessionQueue dispatch, and implicit ordering for thread safety. Several properties (`framesContinuation`, `notificationObservers`, `frameDelegate`) are accessed from multiple contexts without synchronization, relying on call-order guarantees. This works today but is fragile under modification.
- **Double-release in SessionManager.** The `stopSession`-during-`startSession` path results in two `release(for: .session)` calls. The code handles it safely due to `Set.remove` idempotency, but this implicit contract should be documented or explicitly prevented.
- **Polling-based classification loop.** `AppState.startClassificationLoop` polls `lastResult` every 100ms. This adds up to 100ms latency and wastes cycles checking when no frames are available. An observation-based or AsyncStream-based push model would be more efficient, though the current approach is simple and the 100ms overhead is minor for a 10 FPS pipeline.
- **No structured concurrency.** The detection loop, classification loop, and action dispatch loop are all unstructured Tasks stored as properties. Cancellation is manual. A `TaskGroup` or `withTaskGroup` approach would give clearer lifecycle guarantees, though it would require restructuring the service ownership model.

**Overall:** This is a well-architected codebase with intentional concurrency boundaries and careful state machine design. The main risks are in the `CameraManager` threading model (inherent complexity of wrapping GCD-based APIs) and the potential for interleaving during `SessionManager` state transitions (correctly handled but non-obviously). The code reads like it was designed by someone who understands both the AVFoundation threading model and Swift concurrency's sharp edges.