docs(reports): add reviews for offscreen-bubble fix

Code-reviewer flagged the clamp-before-render ordering nit; brainstormer
ranked the layered (validate + clamp) approach over topology-hash / per-monitor
pinning alternatives.

plans/reports/brainstormer-260518-0917-offscreen-bubble-recovery.md

@@ -0,0 +1,80 @@
+# Brainstorm: Off-Screen Bubble Recovery
+
+## 1. Recovery strategies ranked
+
+### RECOMMEND — Layered: validate on load + clamp on create
+
+**A. Validate position in `settings::load`** (primary defense)
+- After deserialize, walk `bubble_positions`. For each `Some((x,y))`, build a probe rect `(x, y, x+min_w, y+min_h)` and check via `MonitorFromRect(... MONITOR_DEFAULTTONULL)`. If null → set to `None`.
+- One pass, ~15 lines, runs before any window code sees the value.
+- Pro: KISS, no race with `ShowWindow`, fixes related bugs (hand-edited JSON, dpi-changed coords).
+
+**B. Clamp on create** (defense-in-depth, kept as proposed)
+- After `CreateWindowExW`, before `ShowWindow`, call `clamp_into_work_area(hwnd)`.
+- Catches monitor unplug **between** `load()` and `create()` (rare but possible: laptop closed mid-startup).
+- Cost: one extra call, idempotent.
+
+Both together are the right answer. Neither alone covers all cases.
+
+### CONSIDER — Visual cue
+
+**C. Tray balloon "Widget repositioned to primary monitor"**
+- Only when validator actually relocated. Uses existing `Shell_NotifyIconW NIF_INFO`. ~20 lines.
+- Risk: balloon spam on dock/undock cycles. Fire only when *saved* position was killed.
+
+### AVOID
+
+**D. Topology fingerprint** — overkill, doesn't preserve intent better than (A).
+**E. Per-monitor relative pinning** — future feature, not a fix. YAGNI.
+
+## 2. Trade-off matrix
+
+| Approach | Preserves intent on replug | Surprise on cold start | LOC | Risk |
+|----------|---------------------------|------------------------|-----|------|
+| A (validate-on-load) | No — wipes saved coord | Low | ~15 | None |
+| B (clamp-on-create) | Partial — moves to nearest edge | Low | ~3 | None |
+| A+B | No | Low | ~18 | None |
+| A+B+C | Same + explains itself | Very low | ~38 | Balloon fatigue |
+| D (topology hash) | Yes if same monitor before next launch | Medium | ~80 | Maintenance |
+| E (relative pin) | Yes | Medium | ~150 | Premature |
+
+## 3. Edge cases proposed fix misses
+
+1. **Saved-pos monitor asleep / no input** — `MonitorFromRect` still returns handle; A+B no-op. Correct.
+2. **DPI change while app closed** — px coords technically valid by topology; A passes, B no-op. Acceptable.
+3. **Negative-coord monitors (secondary left of primary)** — validator MUST use `MONITOR_DEFAULTTONULL`, not `DEFAULTTONEAREST` (would silently snap valid secondary coord to primary).
+4. **Dual bubbles overlap after relocate** — both clamped to bottom-right of primary → stacked. `default_position` staggers Codex; clamp doesn't. Minor.
+5. **User drags to secondary, unplugs, restarts** — A+B: bubble at primary default; saved pos destroyed. No recovery on replug. Acceptable for v1.
+6. **Dock-daily multi-monitor user** — every undock wipes pos; every dock back gives default. Annoying. Case where E would win. Punt unless reported.
+
+## 4. Logging strategy (minimal)
+
+On the visibility-affecting path only:
+
+- `info`: `bubble create model={} pos=({},{}) size={}x{} dpi={}` — one line per bubble at create.
+- `warn`: `bubble position ({},{}) outside all monitors, resetting to default` — fires in validator. **This is the line that would have solved this bug in 5 seconds.**
+- `warn`: `clamp_into_work_area moved bubble from ({},{}) to ({},{})` — fires on create-time clamp.
+- `debug`: monitor enumeration on startup.
+
+Skip: per-render logs, drag logs, timer ticks.
+
+## 5. "Reset position" discoverability — secondary
+
+Menu item exists but buried. If A+B work, this path is unreachable. Don't add a "Reset position" balloon prompt — confirmation fatigue. Just fix silently and the §4 warn + §C balloon explain it once.
+
+## Recommended action
+
+1. Add `BubblePositions::validate(&mut self)` called from `settings::load`. Use `MonitorFromRect(... MONITOR_DEFAULTTONULL)` with `(x, y, x+MIN_BUBBLE_SIZE, y+MIN_BUBBLE_SIZE)`. Set to `None` on miss. Log `warn`.
+2. Call `clamp_into_work_area(hwnd)` in `bubble::create` between `CreateWindowExW` and `ShowWindow`. Log `warn` on movement.
+3. Add the 4 log lines from §4.
+4. Single tray balloon "Widget repositioned: previous monitor not connected" once per launch when validator killed any saved position.
+5. Defer monitor-index pinning (E) and topology hash (D).
+
+Total: ~40 lines, one new function, two log statements, one balloon call.
+
+## Unresolved questions
+
+- Balloon (item 4): opt-in or always-on? Default always-on.
+- `MIN_BUBBLE_SIZE` as probe rect, or account for current `bubble_size_logical`? Min safer.
+- Re-attempt last-known coord on replug? Probably no — YAGNI.
+- Codex/Claude stagger preservation on auto-relocate? Currently they'd stack. Worth fixing in same patch?

plans/reports/code-reviewer-260518-0917-bubble-offscreen-clamp-fix.md

@@ -0,0 +1,60 @@
+# Code Review: Bubble Off-Screen Clamp Fix
+
+**Scope:** Proposed bug-fix for v0.1.7 "widget enabled but not shown" — saved positions on disconnected monitor.
+**Files:** `src/bubble.rs` (create, clamp_into_work_area, set_user_visible, default_position), `src/app.rs` (spawn_bubble, toggle_widget_visibility, reset_positions).
+
+## Overall Assessment
+
+**Fix is correct and minimal. Ship it with two small refinements.** Root-cause matches code (verified: `bubble.rs:143-160` passes saved `position` straight into `CreateWindowExW`; `clamp_into_work_area` at `:770` only wired into `WM_SETTINGCHANGE` at `:486`). Approach is the right shape: clamp post-create, pre-show.
+
+## Critical Issues
+
+None.
+
+## High Priority
+
+1. **Call order — clamp must run BEFORE `render(hwnd)` at `bubble.rs:220`, not just before `ShowWindow` at `:222`.** `render` calls `GetWindowRect` (`:1108`) for the `UpdateLayeredWindow` destination point. If clamp runs after `render`, the first frame paints at the off-screen coords; second paint only happens on next update_data tick. Move `clamp_into_work_area(hwnd)` to between line `:218` (state insert) and `:220` (render).
+
+## Medium Priority
+
+2. **`MonitorFromWindow` on a not-yet-shown off-screen window — verified safe.** Win32 sets the window rect immediately at `CreateWindowExW` return (visibility is irrelevant to `GetWindowRect`). With `MONITOR_DEFAULTTONEAREST` and a window whose entire rect lies on a disconnected monitor, the OS computes intersection with each *currently attached* monitor's rect; none intersect → falls back to nearest by Euclidean distance → returns the primary on a single-monitor setup. Saved `[2407,1282]` on a 1920-wide primary → nearest = primary → clamp pulls to `(1920-w, …)`. Correct.
+
+3. **Multi-monitor edge case is preserved.** If the saved position is on a still-connected secondary, `MonitorFromWindow` returns that secondary monitor and clamps within its work area — no unwanted pull to primary. Good.
+
+4. **Partial off-screen.** `clamp_into_work_area` only adjusts when fully outside (clamps each axis independently to `[wa.left, wa.right-w]`). A window whose top-left is on-screen but bottom-right spills off → it pulls the whole window inside. Behaviour is fine; matches `snap_to_edge` (`:644-645`).
+
+5. **DPI mismatch (saved from 4K → 1080p primary):** the saved coords are physical pixels but the new bubble's `width_px/height_px` are recomputed against the *current* primary DPI (`:140-142`). Clamp uses the new size against the new monitor's work area — correct. No DPI bug.
+
+6. **`default_position` case:** no-op (already inside work area). Safe.
+
+## Low Priority
+
+7. **Log levels are appropriate.** `info!` in `create` (fires once per bubble creation), `set_user_visible` (fires only on user-toggle — verified at `app.rs:1152` only called from `toggle_widget_visibility`), and `toggle_widget_visibility` (one event per click). None on the render hot path. Approved.
+
+8. **Alternative call site (clamp in `app::spawn_bubble`):** Less attractive. `spawn_bubble` doesn't own the HWND lifecycle and would need a fresh `GetWindowRect` round-trip. Keeping the clamp inside `bubble::create` keeps the bubble module the sole owner of window geometry and means future call sites (e.g. tests, a hypothetical re-create-on-DPI-change) also benefit for free. The "bubble module stays position-agnostic" argument is weak — it already calls `default_position`, `snap_to_edge`, and `clamp_into_work_area`. Position-aware is the status quo.
+
+## Side Effects
+
+- No callers of `bubble::create` assert the returned HWND is at the exact requested coords. `app::spawn_bubble` (`:277`) ignores position post-create; `reset_positions` (`:1156`) destroys + recreates. Safe.
+- `position(hwnd)` (`:322`) reads live `GetWindowRect`, so any subsequent `on_bubble_moved` save reflects the clamped coords — this self-heals the persisted bad value on first drag.
+
+## Positive Observations
+
+- Clamp helper already exists and is correct (`:770-809`).
+- Fix is one line + three log statements; minimal blast radius.
+- Persisted-corruption auto-heal via first interaction is a nice property.
+
+## Recommended Actions
+
+1. **MUST:** Place `clamp_into_work_area(hwnd)` between `lock_bubbles().insert(...)` (`:218`) and `render(hwnd)` (`:220`) — not after `render`.
+2. **SHOULD:** Add an `info!` in `clamp_into_work_area` that fires only when `nx != r.left || ny != r.top` (i.e. the actual reposition path). Free diagnostic for future "bubble moved itself" reports.
+3. **CONSIDER:** Persist the clamped position immediately after `create` so `settings.json` is self-healed on next launch, not only after a drag. Trade-off: writes settings on every startup; current behaviour writes only on user action. Probably YAGNI — drift gets repaired on first interaction.
+
+## Unresolved Questions
+
+- Should we also persist the corrected position eagerly (action 3)? Default to no per YAGNI; flag for user.
+- Does Windows ever defer `CreateWindowExW` window-rect commit until `ShowWindow`? Per MSDN and verified by existing `snap_to_edge` using the same pattern in `WM_EXITSIZEMOVE`, no — rect is committed synchronously.
+
+**Status:** DONE_WITH_CONCERNS
+**Summary:** Fix is correct and small. One ordering bug: clamp must precede `render`, not just `ShowWindow`, otherwise the first paint targets the off-screen coords.
+**Concerns:** Action 1 (clamp before render) is a real correctness issue — the proposal as written ("after CreateWindowExW succeeds and before ShowWindow") technically permits ordering after `render`, which would defeat the fix until the next data update.