follow-up(occupancy): P3 cleanups from #115 review (config save) #119

New issue

Open

opened 2026-09-25 19:17:04 +00:00 by gabogg · 0 comments

gabogg commented

2026-09-25 19:17:04 +00:00

Owner

Follow-ups from the pass-2 review of #115 (admin config save). All P3; the P2 (stale offset after a save) and the "config failed to load" guard were fixed in 937141d before merge.

Standards

Static request schema. OccupancyConfigUpdate is built with create_model, so pyright/mypy reject it as a type annotation and editors lose field completion. Consider an explicit class, or a shared base both schemas derive from, so the fields aren't duplicated.
Test the key allowlist. OccupancyRepository.update_config_fields_async raises ValueError for fields outside OccupancyConfigUpdate; nothing tests it.
Dead code. update_config_async has no caller left in app/; only tests use it for seeding. Keep it seed-only (and say so) or replace the seeds with update_config_fields_async / the set_* methods.
Redundant bool→int conversion in update_config_fields_async (sqlite3 stores bools as 0/1).
FORM_FIELDS in tests/test_occupancy_config_api.py still copies the 9 named inputs from app.js by hand.
calibration_mode: str on the response model could be a Literal/enum.

Spec

No JS test for the form's offset bookkeeping (send only when edited; refresh after a save). Only the server side is tested.

🤖 Generated with Claude Code

Follow-ups from the pass-2 review of #115 (admin config save). All P3; the P2 (stale offset after a save) and the "config failed to load" guard were fixed in `937141d` before merge. ## Standards - [ ] **Static request schema.** `OccupancyConfigUpdate` is built with `create_model`, so pyright/mypy reject it as a type annotation and editors lose field completion. Consider an explicit class, or a shared base both schemas derive from, so the fields aren't duplicated. - [ ] **Test the key allowlist.** `OccupancyRepository.update_config_fields_async` raises `ValueError` for fields outside `OccupancyConfigUpdate`; nothing tests it. - [ ] **Dead code.** `update_config_async` has no caller left in `app/`; only tests use it for seeding. Keep it seed-only (and say so) or replace the seeds with `update_config_fields_async` / the `set_*` methods. - [ ] **Redundant bool→int conversion** in `update_config_fields_async` (sqlite3 stores bools as 0/1). - [ ] **`FORM_FIELDS`** in `tests/test_occupancy_config_api.py` still copies the 9 named inputs from `app.js` by hand. - [ ] **`calibration_mode: str`** on the response model could be a `Literal`/enum. ## Spec - [ ] **No JS test** for the form's offset bookkeeping (send only when edited; refresh after a save). Only the server side is tested. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

gabogg added the

ready-for-agent

label

2026-09-25 19:17:04 +00:00

gabogg referenced this issue

2026-09-25 19:17:08 +00:00

fix(occupancy): saving the config form no longer resets calibration #115

gabogg referenced this issue from a commit

2026-09-25 19:17:11 +00:00

fix(occupancy): saving the config form no longer resets calibration (#115)