# Refactor Plan: Per-Provider Typed Device Configs

**Status:** Planned, not started.
**Target branch:** `refactor/device-typed-configs`
**Intended executor:** Sonnet agent (one phase per invocation; human review between phases).

## Goal

Replace the flat [`DeviceInfo`](../../server/src/ledgrab/core/processing/target_processor.py) dataclass (and the `**kwargs`-based `LEDDeviceProvider.create_client(url, **kwargs)` contract) with a **discriminated union of per-provider config dataclasses**. Each provider owns its config type and reads typed fields instead of guessing kwargs.

## Motivation

Current pain points:

- [server/src/ledgrab/core/processing/wled_target_processor.py](../../server/src/ledgrab/core/processing/wled_target_processor.py) unpacks ~21 fields by hand into `create_led_client(**kwargs)`.
- Every provider's `create_client` starts with `kwargs.get("x", default)` — no type safety, no IDE hints, no way to know at a glance which fields a provider actually uses.
- Adding a new per-device-type field requires threading it through `Device` → `DeviceInfo` → `_DEVICE_FIELD_DEFAULTS` → call-site unpacking → kwargs bag → provider.
- Fields leak across device types (a WLED device carries `ble_govee_key=""` at runtime for no reason).

## Scope guardrails

- **Storage schema (SQLite) unchanged.** Columns stay, dead-for-this-type fields stay, no destructive migration.
- **Frontend HTML/TS unchanged in phases 1-4.** It already branches on `device_type` with show/hide logic. Frontend changes are deferred to Phase 5.
- **API schemas are last.** Phase 5 converts `DeviceCreate`/`DeviceUpdate`/`DeviceResponse` to a Pydantic v2 discriminated union. This is the only breaking external change and can be deferred indefinitely if needed.

---

## Phase 1 — Config hierarchy (foundation, non-breaking)

### Create

**File:** `server/src/ledgrab/core/devices/device_config.py`

Pattern:

```python
from dataclasses import dataclass
from typing import List, Literal, Optional, Union

@dataclass(frozen=True)
class BaseDeviceConfig:
    device_id: str
    device_url: str
    led_count: int
    software_brightness: int = 255
    test_mode_active: bool = False
    auto_shutdown: bool = False
    rgbw: bool = False

@dataclass(frozen=True)
class WLEDConfig(BaseDeviceConfig):
    device_type: Literal["wled"] = "wled"
    use_ddp: bool = False

# ... one @dataclass(frozen=True) per provider
```

### Config field inventory

Base: `device_id`, `device_url`, `led_count`, `software_brightness`, `test_mode_active`, `auto_shutdown`, `rgbw`.

| Config         | Extra fields beyond Base |
| -------------- | ------------------------ |
| WLEDConfig     | `use_ddp: bool = False` |
| AdalightConfig | `baud_rate: Optional[int] = None` |
| AmbiLEDConfig  | `baud_rate: Optional[int] = None` |
| DMXConfig      | `dmx_protocol`, `dmx_start_universe`, `dmx_start_channel` |
| ESPNowConfig   | `baud_rate`, `espnow_peer_mac`, `espnow_channel` |
| HueConfig      | `hue_username`, `hue_client_key`, `hue_entertainment_group_id` |
| SPIConfig      | `spi_speed_hz`, `spi_led_type` |
| ChromaConfig   | `chroma_device_type` |
| GameSenseConfig| `gamesense_device_type` |
| BLEConfig      | `ble_family`, `ble_govee_key` |
| GroupConfig    | `group_mode`, `group_device_ids` (**no `device_store` here** — see Phase 2) |
| OpenRGBConfig  | `zone_mode` |
| MockConfig     | `send_latency_ms: int = 0` |
| DemoConfig     | `send_latency_ms: int = 0` |
| MQTTConfig     | (none) |
| WSConfig       | (none) |
| USBHIDConfig   | (none — `hid_usage_page` is parsed from the URL, not config) |

```python
DeviceConfig = Union[
    WLEDConfig, AdalightConfig, AmbiLEDConfig, DMXConfig, ESPNowConfig,
    HueConfig, SPIConfig, ChromaConfig, GameSenseConfig, BLEConfig,
    GroupConfig, MQTTConfig, WSConfig, USBHIDConfig, OpenRGBConfig,
    MockConfig, DemoConfig,
]
```

### Add

**`Device.to_config() -> DeviceConfig`** in [server/src/ledgrab/storage/device_store.py](../../server/src/ledgrab/storage/device_store.py) (around lines 14-97 where `Device` lives).

- Dispatches on `self.device_type`.
- Constructs the right subclass, pulling only relevant columns.
- Ignores columns that don't apply to the type.
- This is the **only** place that knows the flat→typed mapping.

### Do NOT touch in Phase 1

- Provider signatures (still `create_client(self, url, **kwargs)`).
- `create_led_client` factory.
- Any call site.
- `DeviceInfo` itself.

### Acceptance

- New unit test `server/tests/core/devices/test_device_config.py`:
  - For each provider, build a `Device` with that `device_type`, call `to_config()`, assert right subclass and right fields.
  - Edge case: extra/irrelevant Device fields must not leak into the wrong config type.
- `cd server && ruff check src/ tests/ --fix` — green.
- `cd server && py -3.13 -m pytest tests/ --no-cov -q` — green (existing tests untouched, new test passes).
- `cd server && npx tsc --noEmit` — green (no TS impact this phase, just a sanity check).

---

## Phase 2 + Phase 3 — Provider API migration + call-site migration (single PR)

**These must land in one commit** because the provider signature change would otherwise break the 3 call sites immediately.

### Change the abstract base

[server/src/ledgrab/core/devices/led_client.py](../../server/src/ledgrab/core/devices/led_client.py):

```python
class LEDDeviceProvider(ABC):
    @abstractmethod
    def create_client(self, config: DeviceConfig, *, deps: ProviderDeps) -> LEDClient: ...
```

`ProviderDeps` is a tiny new dataclass:

```python
@dataclass(frozen=True)
class ProviderDeps:
    device_store: "DeviceStore"
    # Add future cross-cutting runtime deps here (http_client, etc.)
```

`create_led_client`:

```python
def create_led_client(config: DeviceConfig, *, deps: ProviderDeps) -> LEDClient:
    return get_provider(config.device_type).create_client(config, deps=deps)
```

### Update every provider (17 files)

- Narrow signature per provider: e.g. `WLEDDeviceProvider.create_client(self, config: WLEDConfig, *, deps: ProviderDeps)`.
- Drop all `kwargs.get("x")` lookups — read typed fields directly.
- Providers that don't need `deps` just ignore it.
- **GroupDeviceProvider** is the only current consumer of `deps`: reads `deps.device_store`.

### Call sites (3)

1. [server/src/ledgrab/core/processing/wled_target_processor.py](../../server/src/ledgrab/core/processing/wled_target_processor.py) lines ~120-148 — the 21-field unpacking. Replace with:
   ```python
   config = device.to_config()
   self._led_client = create_led_client(config, deps=self._provider_deps)
   ```
   `self._provider_deps` is plumbed in from `ProcessorManager` when the target processor is constructed.
2. [server/src/ledgrab/core/processing/device_test_mode.py](../../server/src/ledgrab/core/processing/device_test_mode.py) lines 72-78 — minimal test-mode client. Build a synthetic config via a helper `_minimal_config_for_test_mode(device)` (keeps just `device_id`, `device_url`, `led_count`, `baud_rate`) and pass it.
3. [server/src/ledgrab/core/devices/group_client.py](../../server/src/ledgrab/core/devices/group_client.py) lines 47-70 — child client construction inside the group. Same pattern: `child_config = child_device.to_config()`; pass `deps` through.

### Delete

- `DeviceInfo` dataclass in [server/src/ledgrab/core/processing/target_processor.py](../../server/src/ledgrab/core/processing/target_processor.py) lines 71-109.
- `ProcessorManager._get_device_info()` and `_DEVICE_FIELD_DEFAULTS` in [server/src/ledgrab/core/processing/processor_manager.py](../../server/src/ledgrab/core/processing/processor_manager.py) lines 230-275 — `Device.to_config()` subsumes this. Verify no other callers via `ast-index usages "_get_device_info"`.

### Acceptance

- `ast-index search "device_info\."` — no hits in non-test code.
- `ast-index search "DeviceInfo"` — no hits outside archival comments.
- `cd server && py -3.13 -m pytest tests/ --no-cov -q` — all tests pass.
- Manual smoke: start server, create a WLED device, start processing, verify LEDs update (or mock output shows frames).
- `cd server && ruff check src/ tests/ --fix` — green.

---

## Phase 4 — Test migration

Update these files:

- `server/tests/storage/test_device_store.py` — add `to_config()` cases per device type.
- `server/tests/api/routes/test_devices_routes.py` — should be mostly untouched (API schemas still flat until Phase 5).
- `server/tests/e2e/test_device_flow.py` — update internal assertions only if they touch `DeviceInfo` directly.
- `server/tests/test_group_device.py` — construct child clients with `GroupConfig`.
- Any fixture helper that builds a fake `DeviceInfo` — migrate to the right `*Config` subclass.

### Acceptance

- `cd server && py -3.13 -m pytest tests/ --no-cov -q` — all green.
- Coverage of `device_config.py` and `Device.to_config()` ≥ 90%.

---

## Phase 5 — API discriminated union (OPTIONAL, separate PR)

**Do not start until Phases 1-4 are merged and stable.** Flag this to the human before beginning. This is the only phase with an externally breaking change.

### Backend

[server/src/ledgrab/api/schemas/devices.py](../../server/src/ledgrab/api/schemas/devices.py) — replace flat `DeviceCreate`/`DeviceUpdate` with Pydantic v2 tagged unions:

```python
class WLEDDeviceCreate(BaseModel):
    device_type: Literal["wled"]
    name: str
    url: str
    led_count: int
    use_ddp: bool = False
    # ... base fields only

DeviceCreate = Annotated[
    Union[WLEDDeviceCreate, AdalightDeviceCreate, ...],
    Field(discriminator="device_type"),
]
```

Add `model_config = ConfigDict(extra="ignore")` on each union member for **one release cycle** so existing clients (frontend, HAOS integration, curl scripts) that send extra fields don't 422 immediately. Add a deprecation note and tighten to `extra="forbid"` in a follow-up.

### Frontend

- [server/src/ledgrab/static/js/features/devices.ts](../../server/src/ledgrab/static/js/features/devices.ts) and related — when building the POST/PATCH body, scope the payload to the selected `device_type` using the show/hide knowledge already in `device-discovery.ts`.
- **No plain `<select>` elements** — any new pickers use IconSelect or EntitySelect (see root CLAUDE.md UI rules).

### Tests

- Update `test_devices_routes.py` to assert discriminated union rejection of mismatched shapes.
- Add round-trip tests: create device of each type via API → fetch → compare fields.

### Acceptance

- `cd server && py -3.13 -m pytest tests/ --no-cov -q` — green.
- `cd server && npx tsc --noEmit && npm run build` — green.
- Manual smoke for at least 3 device types (WLED, DMX, Hue) — create, edit, delete via UI.
- HAOS integration still works against the server (spot-check; not automated).

---

## Conventions the implementing agent must follow

- **Project task tracker is `TODO.md`** — check the "Refactor: Per-Provider Device Configs" section, tick boxes as phases land. Do **not** use the `TodoWrite` tool.
- **Auto-restart after Python changes.** See [contexts/server-operations.md](../../contexts/server-operations.md).
- **No commits without explicit user approval.** Present each phase's diff for review first.
- **Pre-commit gate every phase:**
  - `cd server && ruff check src/ tests/ --fix`
  - `cd server && py -3.13 -m pytest tests/ --no-cov -q`
  - Phase 5 additionally: `cd server && npx tsc --noEmit && npm run build`
- **No plain `<select>`** — Phase 5 uses IconSelect / EntitySelect.
- **Android parity:** if you add any new runtime dep to `server/pyproject.toml`, update `android/app/build.gradle.kts` per the root [CLAUDE.md](../../CLAUDE.md) "Android Dependency Sync" section. This refactor should not need any new deps.
- **Data migration policy:** storage schema is unchanged, so no JSON-file migration is needed. But if you rename any serialized field during `to_dict`/`from_dict`, add migration logic per the root [CLAUDE.md](../../CLAUDE.md) "Data Migration Policy" section.
- **Use `ast-index`** for code search (`ast-index search`, `ast-index usages`, `ast-index callers`, `ast-index class`). Fall back to Grep only for regex/string-literal/comment searches.
- **Never run `cd` in Bash.** Use absolute paths or the project-relative `cd server && <cmd>` idiom (one-shot, same invocation).

## Known risks

1. **Frozen dataclass + inheritance + defaults** — Python's `@dataclass(frozen=True)` with inheritance requires every subclass field to have a default if any parent field does. Base has defaulted fields. Verify in Phase 1. If it breaks, use `kw_only=True` (Python 3.10+).
2. **`use_ddp` origin** — currently inferred from `self._protocol == "ddp"` at the call site, not from Device storage. Options: add a column (schema change, more work), **or** keep inference logic inside `Device.to_config()` (recommended — no schema change). Prefer the latter.
3. **Test-mode minimal client** ([device_test_mode.py](../../server/src/ledgrab/core/processing/device_test_mode.py) lines 72-78) may not have all `BaseDeviceConfig` fields available. Build a synthetic config via a named helper; do not leak the hack into `Device.to_config()`.
4. **Group `device_store` import cycle** — `GroupConfig` must **not** hold `device_store` (would pull storage into the config module). `ProviderDeps` is the deliberate cut.
5. **BLE optional import** — `BLEDeviceProvider` is conditionally registered (see [led_client.py](../../server/src/ledgrab/core/devices/led_client.py) lines 321-330). Ensure `BLEConfig` still imports cleanly even when `bleak` is absent — put `BLEConfig` in `device_config.py` (not in `ble_provider.py`) so it's always importable.

## Deliverables per phase

1. Branch: `refactor/device-typed-configs`.
2. One commit per phase, conventional-commit messages:
   - `refactor(devices): phase 1 — add DeviceConfig hierarchy`
   - `refactor(devices): phases 2+3 — typed provider signatures + call-site migration`
   - `refactor(devices): phase 4 — test migration to typed configs`
   - `refactor(devices): phase 5 — API discriminated union` (separate PR)
3. Phase-by-phase diffs presented for user review **before** each commit.
4. Final PR body linking all phases, with manual test plan per device type touched.