fix(telemetry): operator door exclusions wiped by restart and telemetry sync — needs AUTO/MANUAL provenance #48

New issue

Closed

opened 2026-09-21 19:51:09 +00:00 by gabogg · 0 comments

gabogg commented

2026-09-21 19:51:09 +00:00

Owner

📌 Problem Statement

door_records.exclude_from_rankings is a single boolean with no provenance, so two different sources write the same bit and become indistinguishable:

AUTO — a newly-seen SENSORLESS_OPEN / SENSORLESS_JUMPERED door is auto-excluded when settings.filter_sensorless is on (app/db/door_repository.py:68-74; default True, app/config.py:45).
MANUAL — an operator quarantines a door via set_exclusion_sync / set_category_exclusion_sync (app/db/door_repository.py:441, :458).

Because both write the same flag, the one rule that reads it — elif category == "VERIFIED_SENSOR" and existing["exclude_from_rankings"] == 1: exclude_from_rankings = 0 (door_repository.py:88-89), plus the startup UPDATE door_records SET exclude_from_rankings = 0 WHERE category = 'VERIFIED_SENSOR' (app/db/database.py:385) — is correct for one source and wrong for the other:

It is the intended auto-recovery for an AUTO-excluded sensorless door that re-classifies to VERIFIED_SENSOR (sensor recovered → re-enter rankings).
It silently wipes an operator's MANUAL exclusion on every server restart and on any telemetry sync whose payload omits the flag.

The MANUAL-wipe is the live bug surfaced in the PR #45 review (operator exclusions do not persist). But the naive fix — deleting both resets — regresses the AUTO path: a sensorless door that recovers would then stay excluded forever, because nothing clears it.

🎯 Required Solution — add provenance, then split the recovery rule

Introduce an exclusion source so AUTO and MANUAL can be governed independently.

Schema (`app/db/database.py`)

Add column exclusion_source TEXT DEFAULT '' to door_records (values: 'AUTO', 'MANUAL', or '' when not excluded).
Add it to the CREATE TABLE block and to the additive PRAGMA table_info migration guard, following the existing is_tracked / tracking_status pattern (database.py:72-87) so existing production DBs upgrade in place.
Backfill on migration: existing exclude_from_rankings = 1 rows get a source by category — VERIFIED_SENSOR → 'MANUAL' (auto never excludes verified doors, so a currently-excluded verified door is operator intent and must be preserved), SENSORLESS_* → 'AUTO'. Rows with exclude_from_rankings = 0 → ''.

Write paths (`app/db/door_repository.py`)

Sensorless auto-exclude (insert, :68-74): when it sets exclude=1, also set exclusion_source='AUTO'.
set_exclusion_sync(excluded=True): set exclusion_source='MANUAL'; excluded=False: clear to ''. Same for set_category_exclusion_sync.
upsert_sync: carry exclusion_source through alongside the flag (read it in the door-record mapping and echo it on write) so routine syncs don't drop it.

Recovery rule (replaces `door_repository.py:88-89` and `database.py:385`)

On a VERIFIED_SENSOR transition, clear the exclusion only when exclusion_source == 'AUTO' (exclude=0, source=''). Leave MANUAL untouched.
Remove the blanket startup UPDATE; the per-door AUTO-recovery rule above covers legitimate re-entry without touching operator exclusions.

Net invariants

Operator (MANUAL) exclusions persist across server restarts (init_db) and across all telemetry syncs.
System (AUTO) sensorless exclusions still auto-clear when the door recovers to VERIFIED_SENSOR.
No blanket reset touches operator intent.

Where this is resolved

Resolve inside PR #45. #45 already owns the exclusion-persistence work; folding the provenance model in there keeps the domain change in one place and one migration, rather than shipping the blanket deletion (which regresses AUTO recovery) and fixing it later. #45's plan item 5 ("Door Ranking Exclusion Invariant") should be updated to this provenance approach instead of deleting the two resets outright.

Surfaced in the PR #45 round-2 review: #45 (comment)
Original persistence bug context: PR #45 review #45 (comment)

✅ Acceptance Criteria

door_records has an exclusion_source column, added via both the CREATE TABLE and the in-place PRAGMA table_info migration; existing DBs upgrade without data loss.
Migration backfills source by category (VERIFIED_SENSOR→MANUAL, SENSORLESS_*→AUTO) for currently-excluded rows.
Sensorless auto-exclude writes source='AUTO'; operator set_exclusion_sync / set_category_exclusion_sync write source='MANUAL'.
A VERIFIED_SENSOR transition clears only AUTO exclusions; MANUAL persists.
Operator-excluded VERIFIED_SENSOR door stays excluded across init_db() (restart) and across a telemetry upsert_sync that omits the flag.
An auto-excluded sensorless door that re-classifies to VERIFIED_SENSOR re-enters rankings.
The blanket startup UPDATE ... WHERE category = 'VERIFIED_SENSOR' and door_repository.py:88-89 are removed, replaced by the source-aware rule.
Backend tests cover both directions (MANUAL persists; AUTO recovers) plus the migration backfill.
CONTEXT.md §2 documents the AUTO vs MANUAL exclusion sources and their lifecycle.

## 📌 Problem Statement `door_records.exclude_from_rankings` is a single boolean with **no provenance**, so two different sources write the same bit and become indistinguishable: - **AUTO** — a newly-seen `SENSORLESS_OPEN` / `SENSORLESS_JUMPERED` door is auto-excluded when `settings.filter_sensorless` is on (`app/db/door_repository.py:68-74`; default `True`, `app/config.py:45`). - **MANUAL** — an operator quarantines a door via `set_exclusion_sync` / `set_category_exclusion_sync` (`app/db/door_repository.py:441`, `:458`). Because both write the same flag, the one rule that reads it — `elif category == "VERIFIED_SENSOR" and existing["exclude_from_rankings"] == 1: exclude_from_rankings = 0` (`door_repository.py:88-89`), plus the startup `UPDATE door_records SET exclude_from_rankings = 0 WHERE category = 'VERIFIED_SENSOR'` (`app/db/database.py:385`) — is correct for one source and wrong for the other: - It **is** the intended auto-recovery for an AUTO-excluded sensorless door that re-classifies to `VERIFIED_SENSOR` (sensor recovered → re-enter rankings). - It **silently wipes** an operator's MANUAL exclusion on every server restart and on any telemetry sync whose payload omits the flag. The MANUAL-wipe is the live bug surfaced in the PR #45 review (operator exclusions do not persist). But the naive fix — deleting both resets — regresses the AUTO path: a sensorless door that recovers would then stay excluded forever, because nothing clears it. ## 🎯 Required Solution — add provenance, then split the recovery rule Introduce an exclusion source so AUTO and MANUAL can be governed independently. ### Schema (`app/db/database.py`) - Add column `exclusion_source TEXT DEFAULT ''` to `door_records` (values: `'AUTO'`, `'MANUAL'`, or `''` when not excluded). - Add it to the `CREATE TABLE` block **and** to the additive `PRAGMA table_info` migration guard, following the existing `is_tracked` / `tracking_status` pattern (`database.py:72-87`) so existing production DBs upgrade in place. - **Backfill** on migration: existing `exclude_from_rankings = 1` rows get a source by category — `VERIFIED_SENSOR` → `'MANUAL'` (auto never excludes verified doors, so a currently-excluded verified door is operator intent and must be preserved), `SENSORLESS_*` → `'AUTO'`. Rows with `exclude_from_rankings = 0` → `''`. ### Write paths (`app/db/door_repository.py`) - Sensorless auto-exclude (insert, `:68-74`): when it sets `exclude=1`, also set `exclusion_source='AUTO'`. - `set_exclusion_sync(excluded=True)`: set `exclusion_source='MANUAL'`; `excluded=False`: clear to `''`. Same for `set_category_exclusion_sync`. - `upsert_sync`: carry `exclusion_source` through alongside the flag (read it in the door-record mapping and echo it on write) so routine syncs don't drop it. ### Recovery rule (replaces `door_repository.py:88-89` and `database.py:385`) - On a `VERIFIED_SENSOR` transition, clear the exclusion **only when `exclusion_source == 'AUTO'`** (`exclude=0`, `source=''`). Leave `MANUAL` untouched. - Remove the blanket startup `UPDATE`; the per-door AUTO-recovery rule above covers legitimate re-entry without touching operator exclusions. ## Net invariants - Operator (`MANUAL`) exclusions persist across server restarts (`init_db`) and across all telemetry syncs. - System (`AUTO`) sensorless exclusions still auto-clear when the door recovers to `VERIFIED_SENSOR`. - No blanket reset touches operator intent. ## Where this is resolved **Resolve inside PR #45.** #45 already owns the exclusion-persistence work; folding the provenance model in there keeps the domain change in one place and one migration, rather than shipping the blanket deletion (which regresses AUTO recovery) and fixing it later. #45's plan item 5 ("Door Ranking Exclusion Invariant") should be updated to this provenance approach instead of deleting the two resets outright. ## Related - Surfaced in the PR #45 round-2 review: https://git.gaboggamer.online/gabogg/hikcentral/pulls/45#issuecomment-943 - Original persistence bug context: PR #45 review https://git.gaboggamer.online/gabogg/hikcentral/pulls/45#issuecomment-882 ## ✅ Acceptance Criteria - [ ] `door_records` has an `exclusion_source` column, added via both the `CREATE TABLE` and the in-place `PRAGMA table_info` migration; existing DBs upgrade without data loss. - [ ] Migration backfills source by category (`VERIFIED_SENSOR`→`MANUAL`, `SENSORLESS_*`→`AUTO`) for currently-excluded rows. - [ ] Sensorless auto-exclude writes `source='AUTO'`; operator `set_exclusion_sync` / `set_category_exclusion_sync` write `source='MANUAL'`. - [ ] A `VERIFIED_SENSOR` transition clears only `AUTO` exclusions; `MANUAL` persists. - [ ] Operator-excluded `VERIFIED_SENSOR` door stays excluded across `init_db()` (restart) and across a telemetry `upsert_sync` that omits the flag. - [ ] An auto-excluded sensorless door that re-classifies to `VERIFIED_SENSOR` re-enters rankings. - [ ] The blanket startup `UPDATE ... WHERE category = 'VERIFIED_SENSOR'` and `door_repository.py:88-89` are removed, replaced by the source-aware rule. - [ ] Backend tests cover both directions (MANUAL persists; AUTO recovers) plus the migration backfill. - [ ] `CONTEXT.md` §2 documents the AUTO vs MANUAL exclusion sources and their lifecycle.

gabogg added the

ready-for-agent

label

2026-09-21 19:51:09 +00:00

gabogg referenced this issue

2026-09-21 19:51:24 +00:00

feat(cli): lightweight db init, credential hygiene, and persistent door exclusions (#41, #43, #48) #45

~~gabogg referenced this issue 2026-09-22 12:35:01 +00:00~~

feat(cli): lightweight db init, credential hygiene, and persistent door exclusions (#41, #43, #48) #45

gabogg referenced this issue from a pull request that will close it,

2026-09-22 12:35:01 +00:00

feat(cli): lightweight db init, credential hygiene, and persistent door exclusions (#41, #43, #48) #45