gabogg/hikcentral
Watch
1
0
Fork
You've already forked hikcentral
 0
Code Issues 113 Pull requests 8 Projects Releases Packages Wiki Activity Actions

feat(doors): implement empirical observational door telemetry and tracking classification #5

Merged
gabogg merged 2 commits from feat/empirical-door-tracking into master 2026-09-05 19:35:37 +00:00
Conversation 2 Commits 2 Files changed 9 +1093 -371
gabogg commented 2026-09-04 18:56:31 +00:00
Owner
Copy link

🚪 Summary of Changes

Context & Motivation

Rather than relying solely on upstream static snapshot status (doorState: 0..4) which can produce misleading metrics due to uninstrumented dummy channels or dry loops, this pull request implements an Empirical Observational Telemetry Model.

The system actively observes and scores door telemetry over time—tracking verified physical transitions, access credential events, push button grants, and magnetic lock feedback—to partition physical doors into verified active channels (Tracked) versus dormant or uninstrumented channels (Untracked).

🔑 Key Architecture & Implementation Details

1. 📊 Domain Classification & Confidence Scoring

  • Added TrackingStatus enum (TRACKED, UNTRACKED, OFFLINE).
  • Implemented classify_empirical_tracking(...) in app/services/door_classifier.py:
    • TRACKED: Doors with observed physical state transitions (N > 0) or confirmed active access cycles receive high empirical confidence scores (0.60 \dots 1.00).
    • UNTRACKED: Unobserved static channels, utility loops, and uninstrumented channels are identified and isolated without polluting operational metrics.
    • OFFLINE: Disconnected or unreachable controllers/channels are flagged cleanly.

2. ⚡ Dynamic Real-Time Promotion

  • In app/services/door_service.py, whenever a live state transition occurs during polling synchronization or a webhook event (DOOR_OPEN, DOOR_CLOSE, CARD_PASS) is ingested, the door is promoted to TRACKED with last_observed_activity recorded.

3. 🗄️ Database & Schema Persistence

  • Updated SQLite schema in app/db/database.py with automated migration checks for:
    • is_tracked (INTEGER DEFAULT 0)
    • tracking_status (TEXT DEFAULT 'UNTRACKED')
    • tracking_confidence (REAL DEFAULT 0.0)
    • tracking_reason (TEXT DEFAULT '')
    • last_observed_activity (REAL)
  • Updated app/db/door_repository.py to persist and read empirical columns across sync and async operations.

4. 📈 Aggregation & Status Partitions

  • Updated DoorOverviewResponse and get_door_overview() to deliver:
    • tracked_doors_count, untracked_doors_count
    • tracked_open_count, tracked_closed_count
    • untracked_open_count, untracked_closed_count
    • tracked_doors, untracked_doors arrays
    • Full backward compatibility for standard open/closed tallies.

5. 🧹 Documentation & Artifacts Maintenance

  • Removed obsolete refactor description documentation (docs/REFACTOR_PR_DESCRIPTION.md) to maintain a clean codebase context.

🧪 Test Suite & Verification

  • Comprehensive test coverage expanded to 64 passing tests (pytest clean):
## 🚪 Summary of Changes ### Context & Motivation Rather than relying solely on upstream static snapshot status (`doorState: 0..4`) which can produce misleading metrics due to uninstrumented dummy channels or dry loops, this pull request implements an **Empirical Observational Telemetry Model**. The system actively observes and scores door telemetry over time—tracking verified physical transitions, access credential events, push button grants, and magnetic lock feedback—to partition physical doors into verified active channels (**Tracked**) versus dormant or uninstrumented channels (**Untracked**). --- ### 🔑 Key Architecture & Implementation Details #### 1. 📊 Domain Classification & Confidence Scoring - Added `TrackingStatus` enum (`TRACKED`, `UNTRACKED`, `OFFLINE`). - Implemented `classify_empirical_tracking(...)` in [`app/services/door_classifier.py`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/services/door_classifier.py): - **`TRACKED`**: Doors with observed physical state transitions ($N > 0$) or confirmed active access cycles receive high empirical confidence scores ($0.60 \dots 1.00$). - **`UNTRACKED`**: Unobserved static channels, utility loops, and uninstrumented channels are identified and isolated without polluting operational metrics. - **`OFFLINE`**: Disconnected or unreachable controllers/channels are flagged cleanly. #### 2. ⚡ Dynamic Real-Time Promotion - In [`app/services/door_service.py`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/services/door_service.py), whenever a live state transition occurs during polling synchronization or a webhook event (`DOOR_OPEN`, `DOOR_CLOSE`, `CARD_PASS`) is ingested, the door is promoted to `TRACKED` with `last_observed_activity` recorded. #### 3. 🗄️ Database & Schema Persistence - Updated SQLite schema in [`app/db/database.py`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/db/database.py) with automated migration checks for: - `is_tracked` (`INTEGER DEFAULT 0`) - `tracking_status` (`TEXT DEFAULT 'UNTRACKED'`) - `tracking_confidence` (`REAL DEFAULT 0.0`) - `tracking_reason` (`TEXT DEFAULT ''`) - `last_observed_activity` (`REAL`) - Updated [`app/db/door_repository.py`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/db/door_repository.py) to persist and read empirical columns across sync and async operations. #### 4. 📈 Aggregation & Status Partitions - Updated [`DoorOverviewResponse`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/schemas/models.py) and `get_door_overview()` to deliver: - `tracked_doors_count`, `untracked_doors_count` - `tracked_open_count`, `tracked_closed_count` - `untracked_open_count`, `untracked_closed_count` - `tracked_doors`, `untracked_doors` arrays - Full backward compatibility for standard open/closed tallies. #### 5. 🧹 Documentation & Artifacts Maintenance - Removed obsolete refactor description documentation (`docs/REFACTOR_PR_DESCRIPTION.md`) to maintain a clean codebase context. --- ### 🧪 Test Suite & Verification - Comprehensive test coverage expanded to **64 passing tests** (`pytest` clean): - Added unit tests for empirical tracking classification across active, offline, static, and utility door profiles in [`tests/test_domain.py`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/tests/test_domain.py). - Added integration tests for overview partitioning and real-time webhook promotion in [`tests/test_doors_reconciliation.py`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/tests/test_doors_reconciliation.py).
gabogg commented 2026-09-05 00:05:43 +00:00
Author
Owner
Copy link

PR Review & Architectural Evaluation Report: feat/empirical-door-tracking

Target PR: feat/empirical-door-tracking vs master (commit 44e4d5b)
Evaluation Verdict: REQUEST_CHANGES ❌ / DO NOT MERGE
Victory Audit: Confirmed & Independently Verified

Executive Summary

An exhaustive, multi-perspective code review and architectural evaluation was conducted across domain modeling, code structure, concurrency, database integrity, and test coverage.

While pytest passes 100% of the existing 64 tests, our adversarial evaluation and empirical stress tests revealed 3 Critical state machine and database defects, 2 Critical concurrency/event-loop starvation bottlenecks, 1 Forensic test-masking integrity issue, and 5 Major architectural flaws.

Evaluation Scorecard

Requirement / Dimension Status Summary
R1. Domain & Architectural Integrity FAIL ❌ Inversion of concerns in DoorRepository, state machine invariant violations in DoorStateManager on network disconnects.
R2. Concurrency, Database & Resilience FAIL ❌ Event-loop starvation via synchronous SQLite writes in 1.0s background loop; race condition between Artemis polling and real-time webhooks.
R3. Test Coverage & Edge Cases FAIL ❌ False-positive unit test masking utility classifier branch; 0% test coverage for sensor diagnostics and offline state transitions.
R4. Actionable Remediation COMPLETE ✅ Complete drop-in code fixes, test fixtures, and architectural patterns provided below.

🔴 Critical Severity Findings (Merge Blockers)

[C1] State Machine Invariant Violation: Network Disconnects Misclassified as Physical Transitions

  • Files: app/services/door_service.py:370-383, app/services/door_service.py:465-478
  • Domain Standard: CONTEXT.md § DoorState & SensorCategory
  • Root Cause: if prev_state is not None and prev_state != new_state: evaluates to True when a door goes offline (doorState = 4) or switches command locks (1 <-> 3).
  • Impact: Dead/unwired doors are elevated to VERIFIED_SENSOR, assigned TRACKED status with 1.0 confidence, and generate phantom DOOR_CLOSE access records upon reconnection (4 -> 1).
  • Remediation:
PHYSICAL_OPEN = (DoorState.OPEN.value, DoorState.REMAIN_OPEN.value)
PHYSICAL_CLOSED = (DoorState.CLOSED.value, DoorState.REMAIN_CLOSED.value)

is_physical_transition = (
    prev_state is not None
    and not is_dev_offline
    and new_state != DoorState.OFFLINE.value
    and prev_state != DoorState.OFFLINE.value
    and (
        (prev_state in PHYSICAL_CLOSED and new_state in PHYSICAL_OPEN)
        or (prev_state in PHYSICAL_OPEN and new_state in PHYSICAL_CLOSED)
    )
)

[C2] Persistence Layer Domain Mutation & Empirical State Demotion

  • Files: app/db/door_repository.py:56-131, app/db/door_repository.py:164-239
  • Root Cause:
    1. DoorRepository.upsert_sync/async executes domain logic (incrementing transitions, forcing TRACKED).
    2. SQL updates fail to coalesce tracking metadata, overwriting verified tracking columns with default fallbacks (UNTRACKED, 0.0, "") when callers pass dictionaries without tracking keys.
  • Impact: Any routine updating door metadata (e.g. ranking exclusion toggles) wipes out verified tracking state in SQLite.
  • Remediation: Strip domain rules from the repository and coalesce incoming values with existing SQLite row columns.

[C3] Event Loop Starvation in 1.0s Background Polling Worker

  • File: app/services/door_service.py:384
  • Root Cause: sync_doors_async() runs on a 1.0-second interval but invokes synchronous door_repo.upsert_sync() inside a 500-door iteration loop, executing hundreds of blocking SQLite transactions directly on the asyncio event loop.
  • Impact: Stalls FastAPI HTTP requests, drops incoming webhooks, and introduces latency spikes.
  • Remediation: Use door_repo.upsert_async() or batch writes offloaded via asyncio.to_thread().

[C4] Async Polling & Webhook Race Condition

  • Files: app/services/door_service.py:304-386, app/services/door_service.py:489-554
  • Root Cause: In sync_doors_async(), execution yields during await artemis.request_async(...). Webhooks arriving during this await update the in-memory state. When the stale polling response resumes, it detects a false transition, triggering spurious DOOR_CLOSE events.
  • Remediation: Guard state updates with timestamp comparison (event_time > last_observed_activity) or synchronize with an asyncio.Lock.

[C5] Forensic Integrity Violation: Masked Test & Unused Facade

  • Files: tests/test_domain.py:191-203, app/db/door_repository.py:148-251
  • Root Cause:
    1. test_door_classifier_empirical_untracked_utility used new_state=OPEN, causing early return at line 60 and completely bypassing the elif is_utility: branch at line 67. The test passed only due to accidental substring overlap.
    2. upsert_async was updated and tested in test_concurrency.py but is dead code in production (never called in door_service.py).
  • Remediation: Fix test input parameters to test the intended code path and integrate upsert_async into async background loops.

🟠 Major Severity Findings

  • [M1] Unjustified Closure Reconciliation Promotes Untracked Doors (door_service.py:628): Sensorless doors forced to TRACKED during access session reconciliation.
  • [M2] Blocking Synchronous Artemis Call in Async Route (app/controllers/door_controller.py:21-27): get_sensor_diagnostics() executes synchronous HTTP calls in an async def FastAPI route.
  • [M3] N+1 SQLite Connection Storms in 1.0s Loop (door_service.py:580, 744, 804): Background workers open and close up to 1,500 fresh SQLite connections every second.
  • [M4] Non-State Webhook Events Trigger False Transitions (door_service.py:516-538): Card scan rejections increment transitions and promote channels to TRACKED.
  • [M5] Dashboard Partitioning Arithmetic Leak (door_service.py:734-750): Offline doors are lumped into untracked_doors, causing inventory discrepancies.

Recommended Next Steps

  1. Apply the physical transition guard to app/services/door_service.py.
  2. Refactor app/db/door_repository.py to coalesce tracking metadata and remove domain logic.
  3. Migrate sync_doors_async() to use upsert_async() with batched database transactions.
  4. Correct the branch-masked test in tests/test_domain.py.
  5. Add unit tests for offline state transitions, sensor diagnostics, and concurrent webhook races.
# PR Review & Architectural Evaluation Report: `feat/empirical-door-tracking` **Target PR**: `feat/empirical-door-tracking` vs `master` (commit `44e4d5b`) **Evaluation Verdict**: **REQUEST_CHANGES ❌ / DO NOT MERGE** **Victory Audit**: **Confirmed & Independently Verified** --- ## Executive Summary An exhaustive, multi-perspective code review and architectural evaluation was conducted across domain modeling, code structure, concurrency, database integrity, and test coverage. While `pytest` passes 100% of the existing 64 tests, our adversarial evaluation and empirical stress tests revealed **3 Critical state machine and database defects, 2 Critical concurrency/event-loop starvation bottlenecks, 1 Forensic test-masking integrity issue, and 5 Major architectural flaws**. --- ## Evaluation Scorecard | Requirement / Dimension | Status | Summary | |---|:---:|---| | **R1. Domain & Architectural Integrity** | **FAIL ❌** | Inversion of concerns in `DoorRepository`, state machine invariant violations in `DoorStateManager` on network disconnects. | | **R2. Concurrency, Database & Resilience** | **FAIL ❌** | Event-loop starvation via synchronous SQLite writes in 1.0s background loop; race condition between Artemis polling and real-time webhooks. | | **R3. Test Coverage & Edge Cases** | **FAIL ❌** | False-positive unit test masking utility classifier branch; 0% test coverage for sensor diagnostics and offline state transitions. | | **R4. Actionable Remediation** | **COMPLETE ✅** | Complete drop-in code fixes, test fixtures, and architectural patterns provided below. | --- ## 🔴 Critical Severity Findings (Merge Blockers) ### [C1] State Machine Invariant Violation: Network Disconnects Misclassified as Physical Transitions - **Files**: [`app/services/door_service.py:370-383`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/services/door_service.py#L370-L383), [`app/services/door_service.py:465-478`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/services/door_service.py#L465-L478) - **Domain Standard**: [`CONTEXT.md`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/CONTEXT.md) § DoorState & SensorCategory - **Root Cause**: `if prev_state is not None and prev_state != new_state:` evaluates to `True` when a door goes offline (`doorState = 4`) or switches command locks (`1 <-> 3`). - **Impact**: Dead/unwired doors are elevated to `VERIFIED_SENSOR`, assigned `TRACKED` status with 1.0 confidence, and generate phantom `DOOR_CLOSE` access records upon reconnection (`4 -> 1`). - **Remediation**: ```python PHYSICAL_OPEN = (DoorState.OPEN.value, DoorState.REMAIN_OPEN.value) PHYSICAL_CLOSED = (DoorState.CLOSED.value, DoorState.REMAIN_CLOSED.value) is_physical_transition = ( prev_state is not None and not is_dev_offline and new_state != DoorState.OFFLINE.value and prev_state != DoorState.OFFLINE.value and ( (prev_state in PHYSICAL_CLOSED and new_state in PHYSICAL_OPEN) or (prev_state in PHYSICAL_OPEN and new_state in PHYSICAL_CLOSED) ) ) ``` --- ### [C2] Persistence Layer Domain Mutation & Empirical State Demotion - **Files**: [`app/db/door_repository.py:56-131`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/db/door_repository.py#L56-L131), [`app/db/door_repository.py:164-239`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/db/door_repository.py#L164-L239) - **Root Cause**: 1. `DoorRepository.upsert_sync/async` executes domain logic (incrementing transitions, forcing `TRACKED`). 2. SQL updates fail to coalesce tracking metadata, overwriting verified tracking columns with default fallbacks (`UNTRACKED`, `0.0`, `""`) when callers pass dictionaries without tracking keys. - **Impact**: Any routine updating door metadata (e.g. ranking exclusion toggles) wipes out verified tracking state in SQLite. - **Remediation**: Strip domain rules from the repository and coalesce incoming values with existing SQLite row columns. --- ### [C3] Event Loop Starvation in 1.0s Background Polling Worker - **File**: [`app/services/door_service.py:384`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/services/door_service.py#L384) - **Root Cause**: `sync_doors_async()` runs on a 1.0-second interval but invokes synchronous `door_repo.upsert_sync()` inside a 500-door iteration loop, executing hundreds of blocking SQLite transactions directly on the asyncio event loop. - **Impact**: Stalls FastAPI HTTP requests, drops incoming webhooks, and introduces latency spikes. - **Remediation**: Use `door_repo.upsert_async()` or batch writes offloaded via `asyncio.to_thread()`. --- ### [C4] Async Polling & Webhook Race Condition - **Files**: [`app/services/door_service.py:304-386`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/services/door_service.py#L304-L386), [`app/services/door_service.py:489-554`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/services/door_service.py#L489-L554) - **Root Cause**: In `sync_doors_async()`, execution yields during `await artemis.request_async(...)`. Webhooks arriving during this await update the in-memory state. When the stale polling response resumes, it detects a false transition, triggering spurious `DOOR_CLOSE` events. - **Remediation**: Guard state updates with timestamp comparison (`event_time > last_observed_activity`) or synchronize with an `asyncio.Lock`. --- ### [C5] Forensic Integrity Violation: Masked Test & Unused Facade - **Files**: [`tests/test_domain.py:191-203`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/tests/test_domain.py#L191-L203), [`app/db/door_repository.py:148-251`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/db/door_repository.py#L148-L251) - **Root Cause**: 1. `test_door_classifier_empirical_untracked_utility` used `new_state=OPEN`, causing early return at line 60 and completely bypassing the `elif is_utility:` branch at line 67. The test passed only due to accidental substring overlap. 2. `upsert_async` was updated and tested in `test_concurrency.py` but is dead code in production (never called in `door_service.py`). - **Remediation**: Fix test input parameters to test the intended code path and integrate `upsert_async` into async background loops. --- ## 🟠 Major Severity Findings - **[M1] Unjustified Closure Reconciliation Promotes Untracked Doors ([`door_service.py:628`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/services/door_service.py#L628))**: Sensorless doors forced to `TRACKED` during access session reconciliation. - **[M2] Blocking Synchronous Artemis Call in Async Route ([`app/controllers/door_controller.py:21-27`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/controllers/door_controller.py#L21-L27))**: `get_sensor_diagnostics()` executes synchronous HTTP calls in an `async def` FastAPI route. - **[M3] N+1 SQLite Connection Storms in 1.0s Loop ([`door_service.py:580, 744, 804`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/services/door_service.py#L580))**: Background workers open and close up to 1,500 fresh SQLite connections every second. - **[M4] Non-State Webhook Events Trigger False Transitions ([`door_service.py:516-538`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/services/door_service.py#L516-L538))**: Card scan rejections increment transitions and promote channels to `TRACKED`. - **[M5] Dashboard Partitioning Arithmetic Leak ([`door_service.py:734-750`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/services/door_service.py#L734-L750))**: Offline doors are lumped into `untracked_doors`, causing inventory discrepancies. --- ## Recommended Next Steps 1. Apply the physical transition guard to [`app/services/door_service.py`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/services/door_service.py). 2. Refactor [`app/db/door_repository.py`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/app/db/door_repository.py) to coalesce tracking metadata and remove domain logic. 3. Migrate `sync_doors_async()` to use `upsert_async()` with batched database transactions. 4. Correct the branch-masked test in [`tests/test_domain.py`](file:///home/gabogg/Trabajo/Orinokia/hikcentral/tests/test_domain.py). 5. Add unit tests for offline state transitions, sensor diagnostics, and concurrent webhook races.
gabogg commented 2026-09-05 00:16:50 +00:00
Author
Owner
Copy link

🛠️ PR Review Feedback Remediated & State Machine Invariants Verified

Thank you for the rigorous, comprehensive review. All Critical findings (C1–C5) and Moderate findings (M1–M5) have been addressed, refactored, and verified with 70 passing automated tests.

🏛️ Critical Architectural & Invariant Remediations

1. [C1] State Machine Transition Invariants Guarded

  • Changes: Defined PHYSICAL_OPEN_STATES = (0, 2) and PHYSICAL_CLOSED_STATES = (1, 3).
  • Implementation: Both sync_doors_async() and handle_webhook_event() strictly require transitions between physical open and physical closed states. Controller disconnections/reconnections (4 <-> 1), offline state transitions, and administrative lock toggles (1 <-> 3) do not increment physical transitions or promote uninstrumented doors to VERIFIED_SENSOR / TRACKED.

2. [C2] Separation of Concerns & SQLite Row Coalescing

  • Changes: Completely excised domain classification and tracking status mutation from door_repository.py.
  • Implementation: Generic updates and exclusion toggles coalesce existing SQLite row columns using COALESCE logic in _prepare_record_values, preserving existing empirical tracking status, confidence scores, and transition counts without risk of field regression.

3. [C3 & M3] Asynchronous Batch Persistence (executemany)

  • Changes: Implemented upsert_batch_sync and upsert_batch_async in door_repository.py utilizing a single database connection and transaction.
  • Implementation: Replaced sequential connection loops during door sync with a single batch execution, eliminating event loop contention, file I/O thrashing, and connection pool starvation.

4. [C4] Polling vs. Real-Time Webhook Concurrency Guard

  • Changes: Polling routines capture poll_start_time before querying upstream Artemis.
  • Implementation: If a real-time webhook update arrives while the asynchronous polling request is in flight (last_observed_activity > poll_start_time), the live in-memory state is preserved, preventing stale polling snapshots from overwriting fresh event data.

5. [C5] Forensic Integrity in Classification & Unmasked Tests

  • Changes: In classify_empirical_tracking, utility door classification (is_utility) is evaluated strictly before open-loop state checks.
  • Implementation: Updated test_door_classifier_empirical_untracked_utility to use new_state=CLOSED to ensure the utility classification path is directly exercised, and added a distinct unit test for uninstrumented open-loop channels.

⚙️ Moderate Improvements Remediated

6. [M1] Invariant-Preserving Upstream Closure Reconciliation

  • Changes: In reconcile_door_states_with_upstream_async(), reconciling unjustified open states to CLOSED maintains existing tracking categorization and confidence rather than forcibly promoting uninstrumented doors to verified channels.

7. [M2] Asynchronous Diagnostics Route

  • Changes: Implemented audit_sensors_vs_maglocks_async() in door_service.py and properly awaited it in door_controller.py, eliminating synchronous blocking inside async FastAPI request handlers.

8. [M4] Webhook Non-State Event Filtering

  • Changes: In handle_webhook_event(), non-state access events (e.g. invalid scans or alarms without door movement) update the activity timestamp without incrementing physical transitions or toggling open/closed state.

9. [M5] Partition Arithmetic Consistency

  • Changes: In get_door_overview(), partition arithmetic strictly separates doors into:
    • offline_doors: is_offline == True
    • tracked_doors: is_tracked == True and not is_offline
    • untracked_doors: is_tracked == False and not is_offline
  • Invariant: tracked_doors_count + untracked_doors_count + offline_count == total_doors holds true with zero category overlap.

✅ Test Suite Verification

  • All 70 test cases across domain classification, concurrency, API routing, reconciliation, batch persistence, and occupancy calculations are passing cleanly (pytest -v).
## 🛠️ PR Review Feedback Remediated & State Machine Invariants Verified Thank you for the rigorous, comprehensive review. All Critical findings (**C1–C5**) and Moderate findings (**M1–M5**) have been addressed, refactored, and verified with **70 passing automated tests**. --- ### 🏛️ Critical Architectural & Invariant Remediations #### 1. [C1] State Machine Transition Invariants Guarded - **Changes**: Defined `PHYSICAL_OPEN_STATES = (0, 2)` and `PHYSICAL_CLOSED_STATES = (1, 3)`. - **Implementation**: Both `sync_doors_async()` and `handle_webhook_event()` strictly require transitions between physical open and physical closed states. Controller disconnections/reconnections (`4 <-> 1`), offline state transitions, and administrative lock toggles (`1 <-> 3`) do **not** increment physical transitions or promote uninstrumented doors to `VERIFIED_SENSOR` / `TRACKED`. #### 2. [C2] Separation of Concerns & SQLite Row Coalescing - **Changes**: Completely excised domain classification and tracking status mutation from `door_repository.py`. - **Implementation**: Generic updates and exclusion toggles coalesce existing SQLite row columns using `COALESCE` logic in `_prepare_record_values`, preserving existing empirical tracking status, confidence scores, and transition counts without risk of field regression. #### 3. [C3 & M3] Asynchronous Batch Persistence (`executemany`) - **Changes**: Implemented `upsert_batch_sync` and `upsert_batch_async` in `door_repository.py` utilizing a single database connection and transaction. - **Implementation**: Replaced sequential connection loops during door sync with a single batch execution, eliminating event loop contention, file I/O thrashing, and connection pool starvation. #### 4. [C4] Polling vs. Real-Time Webhook Concurrency Guard - **Changes**: Polling routines capture `poll_start_time` before querying upstream Artemis. - **Implementation**: If a real-time webhook update arrives while the asynchronous polling request is in flight (`last_observed_activity > poll_start_time`), the live in-memory state is preserved, preventing stale polling snapshots from overwriting fresh event data. #### 5. [C5] Forensic Integrity in Classification & Unmasked Tests - **Changes**: In `classify_empirical_tracking`, utility door classification (`is_utility`) is evaluated strictly before open-loop state checks. - **Implementation**: Updated `test_door_classifier_empirical_untracked_utility` to use `new_state=CLOSED` to ensure the utility classification path is directly exercised, and added a distinct unit test for uninstrumented open-loop channels. --- ### ⚙️ Moderate Improvements Remediated #### 6. [M1] Invariant-Preserving Upstream Closure Reconciliation - **Changes**: In `reconcile_door_states_with_upstream_async()`, reconciling unjustified open states to `CLOSED` maintains existing tracking categorization and confidence rather than forcibly promoting uninstrumented doors to verified channels. #### 7. [M2] Asynchronous Diagnostics Route - **Changes**: Implemented `audit_sensors_vs_maglocks_async()` in `door_service.py` and properly awaited it in `door_controller.py`, eliminating synchronous blocking inside async FastAPI request handlers. #### 8. [M4] Webhook Non-State Event Filtering - **Changes**: In `handle_webhook_event()`, non-state access events (e.g. invalid scans or alarms without door movement) update the activity timestamp without incrementing physical transitions or toggling open/closed state. #### 9. [M5] Partition Arithmetic Consistency - **Changes**: In `get_door_overview()`, partition arithmetic strictly separates doors into: - `offline_doors`: `is_offline == True` - `tracked_doors`: `is_tracked == True and not is_offline` - `untracked_doors`: `is_tracked == False and not is_offline` - **Invariant**: `tracked_doors_count + untracked_doors_count + offline_count == total_doors` holds true with zero category overlap. --- ### ✅ Test Suite Verification - All **70 test cases** across domain classification, concurrency, API routing, reconciliation, batch persistence, and occupancy calculations are passing cleanly (`pytest -v`).
gabogg merged commit 03effac6be into master 2026-09-05 19:35:37 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
blocked
Waits on the issues listed in its "Blocked by:" line before implementation

  
bug
Category: something is broken; behaviour differs from the spec or docs

  
enhancement
Category: new feature or improvement, including refactors, chores and docs

  
high-priority
Address before ordinary-priority work; independent of triage readiness and blockers

  
low-priority
Address after ordinary-priority work; independent of triage readiness and blockers

  
needs-info
Waiting on reporter for more information

  
needs-triage
Maintainer needs to evaluate this issue

  
ready-for-agent
Fully specified, ready for an AFK agent

  
ready-for-human
Requires human implementation

  
referenced
Previously closed issue reopened or referenced by an open issue; visible history marker

  
research
Needs investigation (internet or local resources) before it can be triaged down; used alongside a category and a triage role

  
wontfix
Will not be actioned

No labels
blocked
bug
enhancement
high-priority
low-priority
needs-info
needs-triage
ready-for-agent
ready-for-human
referenced
research
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gabogg/hikcentral!5
No description provided.