# Codebase Review — 2026-02-26

Findings from full codebase review. Items ordered by priority within each category.

## Stability (Critical)

- [x] **Fatal loop exception leaks resources** — Added outer `try/except/finally` with `self._running = False` to all 10 processing loop methods across `live_stream.py`, `color_strip_stream.py`, `effect_stream.py`, `audio_stream.py`, `composite_stream.py`, `mapped_stream.py`. Also added per-iteration `try/except` where missing.
- [x] **`_is_running` flag cleanup** — Fixed via `finally: self._running = False` in all loop methods. *(Race condition via `threading.Event` deferred — current pattern sufficient with the finally block.)*
- [x] **`ColorStripStreamManager` thread safety** — **FALSE POSITIVE**: All access is from the async event loop; methods are synchronous with no `await` points, so no preemption is possible.
- [x] **Audio `stream.stop()` called under lock** — Moved `stream.stop()` outside lock scope in both `release()` and `release_all()` in `audio_capture.py`.
- [x] **WS accept-before-validate** — **FALSE POSITIVE**: All WS endpoints validate auth and resolve configs BEFORE calling `websocket.accept()`.
- [x] **Capture error no-backoff** — Added consecutive error counter with exponential backoff (`min(1.0, 0.1 * (errors - 5))`) in `ScreenCaptureLiveStream._capture_loop()`.
- [ ] **WGC session close not detected** — Deferred (Windows-specific edge case, low priority).
- [x] **`LiveStreamManager.acquire()` not thread-safe** — **FALSE POSITIVE**: Same as ColorStripStreamManager — all access from async event loop, no await in methods.

## Performance (High Impact)

- [x] **Per-pixel Python loop in `send_pixels()`** — Replaced per-pixel Python loop with `np.array().tobytes()` in `ddp_client.py`. Hot path already uses `send_pixels_numpy()`.
- [ ] **WGC 6MB frame allocation per callback** — Deferred (Windows-specific, requires WGC API changes).
- [x] **Gradient rendering O(LEDs×Stops) Python loop** — Vectorized with NumPy: `np.searchsorted` for stop lookup + vectorized interpolation in `_compute_gradient_colors()`.
- [x] **`PixelateFilter` nested Python loop** — Replaced with `cv2.resize` down (INTER_AREA) + up (INTER_NEAREST) — pure C++ backend.
- [x] **`DownscalerFilter` double allocation** — **FALSE POSITIVE**: Already uses single `cv2.resize()` call (vectorized C++).
- [x] **`SaturationFilter` ~25MB temp arrays** — **FALSE POSITIVE**: Already uses pre-allocated scratch buffer and vectorized in-place numpy.
- [x] **`FrameInterpolationFilter` copies full image** — **FALSE POSITIVE**: Already uses vectorized numpy integer blending with image pool.
- [x] **`datetime.utcnow()` per frame** — **LOW IMPACT**: ~1-2μs per call, negligible at 60fps. Deprecation tracked under Backend Quality.
- [x] **Unbounded diagnostic lists** — **FALSE POSITIVE**: Lists are cleared every 5 seconds (~300 entries max at 60fps). Trivial memory.

## Frontend Quality

- [x] **`lockBody()`/`unlockBody()` not re-entrant** — Added `_lockCount` reference counter and `_savedScrollY` in `ui.js`. First lock saves scroll, last unlock restores.
- [x] **XSS via unescaped engine config keys** — **FALSE POSITIVE**: Both capture template and audio template card renderers already use `escapeHtml()` on keys and values.
- [x] **LED preview WS `onclose` not nulled** — Added `ws.onclose = null` before `ws.close()` in `disconnectLedPreviewWS()` in `targets.js`.
- [x] **`fetchWithAuth` retry adds duplicate listeners** — Added `{ once: true }` to abort signal listener in `api.js`.
- [x] **Audio `requestAnimationFrame` loop continues after WS close** — **FALSE POSITIVE**: Loop already checks `testAudioModal.isOpen` before scheduling next frame, and `_cleanupTest()` cancels the animation frame.

## Backend Quality

- [ ] **No thread-safety in `JsonStore`** — Deferred (low risk — all stores are accessed from async event loop).
- [x] **Auth token prefix logged** — Removed token prefix from log message in `auth.py`. Now logs only "Invalid API key attempt".
- [ ] **Duplicate capture/test code** — Deferred (code duplication, not a bug — refactoring would reduce LOC but doesn't fix a defect).
- [x] **Update methods allow duplicate names** — Added name uniqueness checks to `update_template` in `template_store.py`, `postprocessing_template_store.py`, `audio_template_store.py`, `pattern_template_store.py`, and `update_profile` in `profile_store.py`. Also added missing check to `create_profile`.
- [ ] **Routes access `manager._private` attrs** — Deferred (stylistic, not a bug — would require adding public accessor methods).
- [x] **Non-atomic file writes** — Created `utils/file_ops.py` with `atomic_write_json()` helper (tempfile + `os.replace`). Updated all 10 store files.
- [ ] **444 f-string logger calls** — Deferred (performance impact negligible — Python evaluates f-strings very fast; lazy `%s` formatting only matters at very high call rates).
- [x] **`get_source()` silent bug** — Fixed: `color_strip_sources.py:_resolve_display_index()` called `picture_source_store.get_source()` which doesn't exist (should be `get_stream()`). Was silently returning `0` for display index.
- [ ] **`get_config()` race** — Deferred (low risk — config changes are infrequent user-initiated operations).
- [ ] **`datetime.utcnow()` deprecated** — Deferred (functional, deprecation warning only appears in Python 3.12+).
- [x] **Inconsistent DELETE status codes** — Changed `audio_sources.py` and `value_sources.py` DELETE endpoints from 200 to 204 (matching all other DELETE endpoints).

## Architecture (Observations, no action needed)

**Strengths**: Clean layered design, plugin registries, reference-counted stream sharing, consistent API patterns.

**Weaknesses**: No backpressure (slow consumers buffer frames), thread count grows linearly, config global singleton, reference counting races.