From 48dbdb90e9287ceb0e3f798e26713d950f652740 Mon Sep 17 00:00:00 2001
From: "alexei.dolgolyov" <dolgolyov.alexei@gmail.com>
Date: Sat, 23 May 2026 01:22:41 +0300
Subject: [PATCH] docs(review-todo): check off items addressed in 2026-05-23
 autonomous pass

Mark devices.py PATCH fix, WLED route-level test, IPv6 regression
test, IconSelect XSS audit, PEP-604 sweep, magic-number constants,
api/auth except specificity, and the (window as any) static-access
cleanup as done. Defer items are unchanged: performance items keep
their "profile first" caveat, Hue cert pinning + CSP keep the design-
sensitive note, architecture refactors keep the multi-day banner,
and i18n parity is now annotated with the exact missing-key counts
(328 ru / 325 zh) so the next translator pass has a clear scope.
---
 REVIEW_TODO.md | 114 ++++++++++++++++++++++++++++---------------------
 1 file changed, 66 insertions(+), 48 deletions(-)

diff --git a/REVIEW_TODO.md b/REVIEW_TODO.md
index d325e12..dc6664e 100644
--- a/REVIEW_TODO.md
+++ b/REVIEW_TODO.md
@@ -18,6 +18,40 @@ redirects, single source of truth for IP classification in
 `utils/net_classify.py`, allowlist + parity test for inbound WS events,
 typed `Window` globals, and more).
 
+## Items completed in the follow-up autonomous pass (2026-05-23)
+
+- [x] **devices.py PATCH-without-url processor desync** — `update_device`
+      now falls back to `existing.url` so a rename / icon-only edit
+      always tells the processor the current address.
+- [x] **WLED scheme integration test** on `/api/v1/devices` — covers
+      bare IPv4 (`http://`), public hostname (`https://`), and trailing-slash
+      normalisation; lives in `tests/api/routes/test_devices_routes.py`.
+- [x] **IPv6 regression test** — `tests/test_url_scheme.py` now pins
+      public IPv6 → `https://`, ULA → `http://`, and documents the
+      Python-`ipaddress` documentation-prefix classification quirk.
+- [x] **IconSelect XSS audit + defence-in-depth** — every caller
+      audited (all feed `icon` from constants or lookup tables); added
+      `sanitiseIcon` that rejects `<script>`, `javascript:`, `on*=`,
+      `<iframe>`, `<embed>`, `<object>` and warns to the console.
+- [x] **`Optional[T]` → `T | None` (PEP 604)** — 55 sites cleaned via
+      `ruff --fix UP007`. The remaining `Union[…]` aliases for
+      pixel/colour/device-config typing converted by hand. `UP007` now
+      lives in `pyproject.toml` so the rule fires on new code.
+- [x] **Hot-path magic numbers → named constants** — `processed_stream`
+      gains `_FILTER_RECHECK_EVERY_N_FRAMES`; `wled_target_processor`
+      gains `_SKIP_REPOLL_SLEEP_SECONDS`, `_DIAGNOSTICS_REPORT_INTERVAL_SECONDS`,
+      `_CSPT_RECHECK_EVERY_N_ITERATIONS`.
+- [x] **`api/auth.py` `except Exception` tightening** — every WS send /
+      close site is now `except _WS_SEND_BENIGN_EXC` (a narrow tuple of
+      WebSocketDisconnect / RuntimeError / ConnectionError / OSError).
+      The auth-receive path catches the same set plus a final
+      `logger.exception` catch-all for observability on truly unexpected
+      shapes.
+- [x] **`(window as any)` cleanup** — 59 static-property accesses
+      migrated to typed `window.<name>` against `global-types.d.ts`. The
+      remaining 7 sites use dynamic string indexing (`window[fnName]`)
+      and intentionally keep the cast (documented in the typedef file).
+
 ---
 
 ## Architecture refactors (multi-day — own session)
@@ -92,12 +126,8 @@ typed `Window` globals, and more).
       because the UI uses inline event handlers / Jinja templates.
       Mis-set CSP would break the app silently. Defer until templates can
       move to event-delegated handlers, then add a strict policy.
-- [ ] **`api/auth.py` exception specificity** — 9 `except Exception:`
-      sites. Most are intentional best-effort `websocket.send_json`
-      swallows (the WS is already closed or about to be), but the auth
-      decision path itself could be tightened to specific types
-      (`jwt.InvalidTokenError`, `OSError`) + `logger.exception` for
-      observability.
+- [x] **`api/auth.py` exception specificity** — done in the 2026-05-23
+      pass; see top of file.
 - [ ] **Hue bridge cert pinning** — `httpx.AsyncClient(verify=False)` for
       Hue bridge (self-signed cert by design). Should record the
       certificate fingerprint at pairing time and pin it on subsequent
@@ -105,58 +135,46 @@ typed `Window` globals, and more).
 
 ## Mechanical / code-quality (low risk, high line-count)
 
-- [ ] **i18n parity** — **328** keys missing in `ru.json`, **325** missing
-      in `zh.json`. Examples: `section.hide`, `filters.hsl_shift`,
-      `filters.contrast`, `filters.temporal_blur`,
-      `filters.audio_filter_template.desc`. Russian and Chinese users
-      currently see raw keys for these. This is translation work, not
-      code work.
-- [ ] **`Optional[T]` → `T | None`** (PEP 604) — large mechanical refactor
-      across the codebase. Can be auto-fixed via `ruff check --fix
-      --select UP007`. Worth doing once the file splits land.
+- [ ] **i18n parity** — confirmed **328** keys missing in `ru.json` and
+      **325** missing in `zh.json` against the canonical English file.
+      Translation work — needs a native speaker, not a machine-translation
+      pass. Run `py scripts/diff_locale_keys.py` (or copy the diff block
+      out of the 2026-05-23 pass log) to get the exact key list.
+- [x] **`Optional[T]` → `T | None`** — done; `UP007` now enforced via
+      `pyproject.toml` so the rule prevents regressions.
 - [ ] **Hot-path `logger.error(f"...")` → `logger.error("... %s", e)`**
-      lazy-eval — mostly cosmetic; ~200 sites. The f-string still builds
-      the message even when DEBUG is off.
-- [ ] **Remaining `(window as any)` sites** — typed `global-types.d.ts`
-      is in place and new code uses `window.foo` directly, but ~80
-      existing sites still have the cast. Per-site mechanical cleanup.
-      Add `eslint`-equivalent guard (TS rule) to prevent new ones.
-- [ ] **Magic numbers → named constants** in processing hot paths —
-      `_FILTER_RECHECK_EVERY_N_FRAMES = 30` in
-      `core/processing/processed_stream.py:159`; `5 ms` / `5 s` /
-      `30 iterations` literals in `wled_target_processor.py:890,893,915`.
-- [ ] **Standardise `from __future__ import annotations`** across the
-      codebase. Some modules use the future-annotation form, others stick
-      with `Optional[...]`. Enforce one via ruff `FA` rules.
+      lazy-eval — 658 sites flagged by `ruff --select G004`. Deferred
+      because it is genuinely cosmetic at ERROR level (always emitted)
+      and the cumulative cost is negligible. Worth doing if/when ruff
+      gains a safe autofix, or as a Codemod in a dedicated session.
+- [x] **Remaining `(window as any)` sites** — 59 migrated to typed
+      `window.<name>` access; the 7 surviving sites use dynamic string
+      indexing and are documented as the legitimate exception.
+- [x] **Magic numbers → named constants** — done; see `processed_stream`
+      and `wled_target_processor` constants at the top of each module.
+- [ ] **Standardise `from __future__ import annotations`** — partially
+      mooted by the UP007 cleanup. Files that previously relied on
+      `Optional`/`Union` no longer need the future import; the few that
+      already use `__future__` keep it for forward-reference convenience.
+      A blanket policy would still help — leave as a stylistic followup.
 
 ## Test gaps
 
-- [ ] **Route-level integration test** for the WLED scheme inference —
-      POST `/api/v1/devices` with `{"url": "192.168.1.42",
-      "device_type": "wled"}` and assert the stored device has
-      `url == "http://192.168.1.42"`. The helper is exhaustively
-      unit-tested but no integration test exercises the create/update
-      flow end-to-end.
-- [ ] **IPv6 public address regression** — extend `test_url_scheme.py`
-      with explicit assertions for `2001:db8::1` and similar public IPv6
-      literals (the bare-label fallback used to misclassify these). The
-      helper does the right thing today via the IPv6 probe added during
-      the hardening pass, but no test pins it.
+- [x] **Route-level integration test** for the WLED scheme inference —
+      done; covers create + update in `tests/api/routes/test_devices_routes.py::TestWLEDSchemeInference`.
+- [x] **IPv6 public address regression** — done; pinned in
+      `tests/test_url_scheme.py` for both bracketless and bracketed forms.
 
 ## Pre-existing issues surfaced during the audit (not in our diff)
 
 These were flagged by the auditors but predate the review session — kept
 here as a future-work backlog:
 
-- [ ] **`icon-select.ts:_buildGrid` `item.icon` is interpolated raw** —
-      documented as "trusted SVG by design". If callers ever feed
-      user-supplied icon strings, that's an XSS sink. Audit every caller
-      that builds `IconSelectItem.icon` from non-constant data and
-      reject HTML there.
-- [ ] **`devices.py:461` `manager.update_device_info(device_url=update_data.url)`**
-      receives `None` when a PATCH omits `url` (rename / icon-only edit).
-      The processor never re-syncs in that case. Should pass
-      `existing.url` (after normalization) or skip the call.
+- [x] **`icon-select.ts:_buildGrid` `item.icon` is interpolated raw** —
+      audited; all callers pass project-owned literals or table-lookup
+      results. Added a runtime sanitiser as defence-in-depth.
+- [x] **`devices.py` `manager.update_device_info(device_url=update_data.url)`**
+      None-on-PATCH path — fixed; now falls back to `existing.url`.
 - [ ] **`asyncio.gather` over uncapped client lists** in preview broadcasts
       — slow clients block the loop. Already noted under Performance
       above; pre-existing.