# Acceptance Auditor — Story 4.2 Code Review

You are an **Acceptance Auditor**. Review the diff against the spec and context docs. Check for: violations of acceptance criteria, deviations from spec intent, missing implementation of specified behavior, and contradictions between spec constraints and actual code.

## Instructions

- Compare the diff (below) against the Story 4.2 spec (included below).
- Also consider the context docs: the spec references patterns from Story 4.1 code.
- Output findings as a Markdown list.
- Each finding: **one-line title**, which AC or constraint it violates, and evidence from the diff.
- If all acceptance criteria are met correctly, say "All acceptance criteria satisfied."

---

## Spec: Story 4.2 — Custom Portrait Fallback

**AC-1:** Portrait Fallback section in Privacy Panel — contains file picker, preview, remove option. *(UI — not in this diff)*
**AC-2:** File picker accepts PNG, JPG, WEBP, static GIF. *(Contract validation covered)*
**AC-3:** Unsupported formats rejected with error message. *(Contract validation covered)*
**AC-4:** Custom fallback displayed when `never-connected` state. *(PortraitFallbackHandler logic covered)*
**AC-5:** Custom fallback displayed when `cam-lost` state. *(PortraitFallbackHandler logic covered)*
**AC-6:** No custom portrait → FoundryVTT avatar → system placeholder. *(PortraitFallbackHandler fallback chain covered)*
**AC-7:** Portrait persistence across sessions. *(User flag storage covered)*
**AC-8:** Remove custom image with confirmation. *(removePortraitFallback method covered)*
**AC-9:** Correct dimensions, aspect ratio maintained, no distortion. *(CSS styling — not in this diff)*

### Functional Requirements
- **FR-8:** Portrait Fallback displayed when camera unavailable (never-connected/cam-lost); default is FoundryVTT avatar → system placeholder; same dimensions as live feed.
- **FR-26:** Custom Portrait Fallback via file picker; accepted formats: PNG, JPG, WEBP, static GIF; falls back to FoundryVTT avatar → system placeholder.

### Key Spec Constraints (from story file)
- Storage: DataURL in user flag for v1.0
- File validation: MIME type AND file content validation
- MAX_PORTRAIT_SIZE: 5MB
- `PortraitFallbackHandler` in `src/core/`, `import` from contracts/utils only
- `PlayerPrivacyManager` extensions stay in core/
- Import boundaries enforced
- Validated portrait DataURL before storage
- Silent fallback: if custom portrait fails, fall back to FoundryVTT avatar → system placeholder silently
- Portrait is ONLY displayed in `never-connected` or `cam-lost` states
- `setPortraitFallback` dedicated method, not through `setSetting`
- `getPortraitFallback` returns DataURL string or null
- `removePortraitFallback` removes flag
- `getPortraitFallbackDataURL` convenience method

## Diff

=== MODIFIED FILES ===

```diff
diff --git a/src/contracts/privacy-settings.js b/src/contracts/privacy-settings.js
index b838445..f618241 100644
--- a/src/contracts/privacy-settings.js
+++ b/src/contracts/privacy-settings.js
@@ -5,15 +5,34 @@
  * their on-screen presence. Settings are stored as user flags on the user document.
  *
  * Storage key: game.user.setFlag('video-view-manager', key, value)
- * Shape: { reactionCamEnabled: boolean, hpReactiveCamStylingEnabled: boolean }
+ * Shape: { reactionCamEnabled: boolean, hpReactiveCamStylingEnabled: boolean, customPortraitFallback: string | null }
  *
  * @module contracts/privacy-settings
  */
 
+export const MAX_PORTRAIT_SIZE = 5 * 1024 * 1024;
+
+export const VALID_PORTRAIT_FORMATS = Object.freeze([
+  "image/png",
+  "image/jpeg",
+  "image/webp",
+  "image/gif",
+]);
+
 /**
  * @typedef {Object} PrivacySettings
  * @property {boolean} reactionCamEnabled
  * @property {boolean} hpReactiveCamStylingEnabled
+ * @property {string|null} customPortraitFallback
  */
 
 export const PRIVACY_SETTINGS_VERSION = 1;
@@ -25,6 +44,7 @@ export const PRIVACY_SETTINGS_VERSION = 1;
 export const PRIVACY_SETTINGS_DEFAULT = {
   reactionCamEnabled: false,
   hpReactiveCamStylingEnabled: false,
+  customPortraitFallback: null,
 };
 
@@ -34,6 +54,7 @@ export const PRIVACY_SETTING_KEYS = Object.freeze([
   "reactionCamEnabled",
   "hpReactiveCamStylingEnabled",
+  "customPortraitFallback",
 ]);
 
@@ -45,6 +66,49 @@ export const FEATURE_NAME_MAP = Object.freeze({
   hpReactiveCamStyling: "hpReactiveCamStylingEnabled",
 });
 
+export function validatePortraitDataURL(dataURL) {
+  if (dataURL === null || dataURL === undefined) {
+    return dataURL;
+  }
+  if (typeof dataURL !== "string") {
+    throw new TypeError(`Invalid DataURL: expected string, got ${typeof dataURL}`);
+  }
+  if (dataURL === "") {
+    return dataURL;
+  }
+
+  if (!dataURL.startsWith("data:")) {
+    throw new TypeError("Invalid DataURL format: must start with 'data:'");
+  }
+
+  const mimeMatch = dataURL.match(/^data:(image\/[a-zA-Z0-9+\-.]+|video\/[a-zA-Z0-9+\-.]+)/);
+  if (!mimeMatch) {
+    throw new TypeError("Invalid DataURL format: missing or invalid MIME type");
+  }
+
+  const mimeType = mimeMatch[1].toLowerCase();
+
+  if (!VALID_PORTRAIT_FORMATS.includes(mimeType)) {
+    throw new TypeError(
+      `Unsupported portrait format: ${mimeType}. Supported: ${VALID_PORTRAIT_FORMATS.join(", ")}`
+    );
+  }
+
+  return dataURL;
+}
+
 /**
  * Creates a new PrivacySettings object with defaults.
  */
@@ -63,6 +127,7 @@ export function createPrivacySettings(overrides = {}) {
 
 /**
  * Validates a PrivacySettings DTO. Throws TypeError on any violation.
+ * Backward compatible: accepts settings without customPortraitFallback key.
  */
@@ -75,23 +140,50 @@ export function isValidPrivacySettings(data) {
     throw new TypeError("PrivacySettings: must be an object");
   }
   const obj = /** @type {Record<string, unknown>} */ (data);
-  const { reactionCamEnabled, hpReactiveCamStylingEnabled, ...rest } = obj;
-  if (Object.keys(rest).length > 0) {
+
+  const knownKeys = new Set(PRIVACY_SETTING_KEYS);
+  const unknownKeys = Object.keys(obj).filter((k) => !knownKeys.has(k));
+  if (unknownKeys.length > 0) {
     throw new TypeError(
-      `PrivacySettings: unknown keys: ${Object.keys(rest).join(", ")}`
+      `PrivacySettings: unknown keys: ${unknownKeys.join(", ")}`
     );
   }
-  if (typeof reactionCamEnabled !== "boolean") {
-    throw new TypeError(
-      `PrivacySettings: reactionCamEnabled must be a boolean, got ${typeof reactionCamEnabled}`
-    );
+
+  if ("reactionCamEnabled" in obj) {
+    if (typeof obj.reactionCamEnabled !== "boolean") {
+      throw new TypeError(
+        `PrivacySettings: reactionCamEnabled must be a boolean, got ${typeof obj.reactionCamEnabled}`
+      );
+    }
   }
-  if (typeof hpReactiveCamStylingEnabled !== "boolean") {
-    throw new TypeError(
-      `PrivacySettings: hpReactiveCamStylingEnabled must be a boolean, got ${typeof hpReactiveCamStylingEnabled}`
-    );
+  if ("hpReactiveCamStylingEnabled" in obj) {
+    if (typeof obj.hpReactiveCamStylingEnabled !== "boolean") {
+      throw new TypeError(
+        `PrivacySettings: hpReactiveCamStylingEnabled must be a boolean, got ${typeof obj.hpReactiveCamStylingEnabled}`
+      );
+    }
   }
-  return /** @type {PrivacySettings} */ (data);
+  if ("customPortraitFallback" in obj) {
+    if (obj.customPortraitFallback !== null && typeof obj.customPortraitFallback !== "string") {
+      throw new TypeError(
+        `PrivacySettings: customPortraitFallback must be a string or null, got ${typeof obj.customPortraitFallback}`
+      );
+    }
+    if (typeof obj.customPortraitFallback === "string" && obj.customPortraitFallback !== "") {
+      try {
+        validatePortraitDataURL(obj.customPortraitFallback);
+      } catch (e) {
+        throw new TypeError(
+          `PrivacySettings: customPortraitFallback ${e.message}`
+        );
+      }
+    }
+  }
+
+  return /** @type {PrivacySettings} */ (obj);
 }
 
diff --git a/src/core/PlayerPrivacyManager.js b/src/core/PlayerPrivacyManager.js
index 117afa2..9347edd 100644
--- a/src/core/PlayerPrivacyManager.js
+++ b/src/core/PlayerPrivacyManager.js
@@ -15,9 +15,11 @@ import {
   PRIVACY_SETTINGS_DEFAULT,
   PRIVACY_SETTING_KEYS,
   FEATURE_NAME_MAP,
+  MAX_PORTRAIT_SIZE,
   validateSettingKey,
   validateSettingValue,
   validateFeatureName,
+  validatePortraitDataURL,
 } from "../contracts/privacy-settings.js";
 
@@ -118,12 +120,19 @@ export class PlayerPrivacyManager {
    */
   async setSetting(userId, key, value) {
+    // Reject customPortraitFallback - use dedicated method instead
+    if (key === "customPortraitFallback") {
+      throw new TypeError(
+        "PlayerPrivacyManager: customPortraitFallback must use setPortraitFallback() method"
+      );
+    }
+
     // Validate key
     validateSettingKey(key);
 
@@ -245,6 +254,149 @@ export class PlayerPrivacyManager {
     }
   }
 
+  /**
+   * Sets a custom portrait fallback DataURL for a user.
+   */
+  async setPortraitFallback(userId, dataURL) {
+    // Validate DataURL format
+    validatePortraitDataURL(dataURL);
+
+    // Get user
+    const user = this._adapter.users.get(userId);
+    if (!user) {
+      throw new TypeError(
+        `PlayerPrivacyManager: User '${userId}' not found`
+      );
+    }
+
+    // Validate user has setFlag method
+    if (typeof user.setFlag !== "function") {
+      throw new TypeError(
+        `PlayerPrivacyManager: User '${userId}' does not support setFlag`
+      );
+    }
+
+    // Get previous value for change event
+    const previousValue = this.getPortraitFallback(userId);
+
+    // Persist the setting via user flag
+    await user.setFlag("video-view-manager", "customPortraitFallback", dataURL);
+
+    // Notify subscribers
+    this._notifyPortraitChange(userId, dataURL, previousValue);
+  }
+
+  /**
+   * Retrieves the custom portrait fallback DataURL for a user.
+   */
+  getPortraitFallback(userId) {
+    const user = this._adapter.users.get(userId);
+
+    // Return null if user doesn't exist or has no getFlag
+    if (!user || typeof user.getFlag !== "function") {
+      return null;
+    }
+
+    const dataURL = user.getFlag("video-view-manager", "customPortraitFallback");
+
+    // Validate the stored DataURL (defensive programming)
+    if (dataURL !== null && dataURL !== undefined) {
+      try {
+        if (typeof dataURL === "string" && dataURL !== "") {
+          validatePortraitDataURL(dataURL);
+        }
+        return dataURL ?? null;
+      } catch (e) {
+        console.warn(
+          `[ScryingPool] PlayerPrivacyManager: Invalid stored portrait DataURL for user '${userId}': ${e.message}`
+        );
+        return null;
+      }
+    }
+
+    return null;
+  }
+
+  /**
+   * Convenience method to get portrait fallback as DataURL directly.
+   */
+  getPortraitFallbackDataURL(userId) {
+    return this.getPortraitFallback(userId);
+  }
+
+  /**
+   * Removes the custom portrait fallback for a user.
+   */
+  async removePortraitFallback(userId) {
+    const user = this._adapter.users.get(userId);
+    if (!user) {
+      throw new TypeError(
+        `PlayerPrivacyManager: User '${userId}' not found`
+      );
+    }
+
+    if (typeof user.unsetFlag !== "function") {
+      throw new TypeError(
+        `PlayerPrivacyManager: User '${userId}' does not support unsetFlag`
+      );
+    }
+
+    const previousValue = this.getPortraitFallback(userId);
+
+    await user.unsetFlag("video-view-manager", "customPortraitFallback");
+
+    this._notifyPortraitChange(userId, null, previousValue);
+  }
+
+  _notifyPortraitChange(userId, newValue, previousValue) {
+    for (const callback of this._subscribers) {
+      try {
+        callback(userId, "customPortraitFallback", newValue, previousValue);
+      } catch (err) {
+        console.error(
+          `[ScryingPool] PlayerPrivacyManager portrait subscriber error:`,
+          err
+        );
+      }
+    }
+  }
```

=== NEW FILES ===

**src/core/PortraitFallbackHandler.js** — Full file available at `/home/morr/work/foundryvtt/video-view-manager/src/core/PortraitFallbackHandler.js`

**tests/helpers/playerPrivacyManagerMock.js** — Full file available at `/home/morr/work/foundryvtt/video-view-manager/tests/helpers/playerPrivacyManagerMock.js`