gabogg/hikcentral

Watch

Fork 0

refactor(telemetry): defensive activity stream exclusion filtering (#42) #44

Merged

gabogg merged 2 commits from refactor/issue-42-defensive-activity-filtering into master

2026-09-21 18:36:30 +00:00

gabogg commented

2026-09-21 18:28:58 +00:00

Owner

Resolves #42.

Problem Statement

In Issue #42, an architectural question was raised regarding duplicate exclusion filtering for recent_activity:

Backend (app/services/door_service.py:1255-1265) filters recent_cycles and stamps exclude_from_rankings on each cycle dictionary.
Frontend (app/static/js/src/ui/command_deck_adapter.js:527-539) cross-checks against snapshot.doors by compiling an excludedCodes Set and dropping cycles where isDoorExcluded(c) or excludedCodes.has(code) is true.

Per mandate to keep the frontend defensive, this PR formally documents the division of responsibility and adds full unit test coverage proving what the defensive check defends against.

Architectural Approach

Documented Ownership (CONTEXT.md):
- Backend (app/services/door_service.py) is the authoritative owner for filtering excluded doors from recent_activity and stamping cycles.
- Frontend adapter (app/static/js/src/ui/command_deck_adapter.js) serves as a defense-in-depth guard: by cross-checking against excludedCodes derived from snapshot.doors, it protects against unstamped legacy cycles, cached client state, replayed payloads, or cycles arriving before background overview synchronization from leaking into the visual activity stream.
Defensive Cross-Check Preserved (command_deck_adapter.js):
- Retained the defensive filter and added inline architectural documentation pointing to CONTEXT.md and Issue #42.
Defensive Unit Test Coverage (test_command_deck_adapter.test.js):
- Added CommandDeckAdapter - Live Activity Stream Defensive Exclusion Filtering for Unstamped Legacy Cycles.
- Directly proves that an unstamped legacy cycle (missing exclude_from_rankings and is_excluded) belonging to an excluded door in snapshot.doors is blocked from rendering, while legitimate cycles for active doors render successfully.
- Verifies edge cases including stamped cycles missing from door inventory and alternate identifier keys (doorId, door_id, door_index_code).

Verification Evidence

Frontend tests: node --test tests/frontend/*.test.js -> 59 passed (100% green).
Backend tests: pytest -> 198 passed (100% green).
Linter & formatting: ruff check . and ruff format --check . -> all checks passed.

Resolves #42. ### Problem Statement In Issue #42, an architectural question was raised regarding duplicate exclusion filtering for `recent_activity`: - Backend (`app/services/door_service.py:1255-1265`) filters `recent_cycles` and stamps `exclude_from_rankings` on each cycle dictionary. - Frontend (`app/static/js/src/ui/command_deck_adapter.js:527-539`) cross-checks against `snapshot.doors` by compiling an `excludedCodes` Set and dropping cycles where `isDoorExcluded(c)` or `excludedCodes.has(code)` is true. Per mandate to keep the frontend defensive, this PR formally documents the division of responsibility and adds full unit test coverage proving what the defensive check defends against. ### Architectural Approach 1. **Documented Ownership (`CONTEXT.md`)**: - Backend (`app/services/door_service.py`) is the authoritative owner for filtering excluded doors from `recent_activity` and stamping cycles. - Frontend adapter (`app/static/js/src/ui/command_deck_adapter.js`) serves as a defense-in-depth guard: by cross-checking against `excludedCodes` derived from `snapshot.doors`, it protects against unstamped legacy cycles, cached client state, replayed payloads, or cycles arriving before background overview synchronization from leaking into the visual activity stream. 2. **Defensive Cross-Check Preserved (`command_deck_adapter.js`)**: - Retained the defensive filter and added inline architectural documentation pointing to `CONTEXT.md` and Issue #42. 3. **Defensive Unit Test Coverage (`test_command_deck_adapter.test.js`)**: - Added `CommandDeckAdapter - Live Activity Stream Defensive Exclusion Filtering for Unstamped Legacy Cycles`. - Directly proves that an unstamped legacy cycle (missing `exclude_from_rankings` and `is_excluded`) belonging to an excluded door in `snapshot.doors` is blocked from rendering, while legitimate cycles for active doors render successfully. - Verifies edge cases including stamped cycles missing from door inventory and alternate identifier keys (`doorId`, `door_id`, `door_index_code`). ### Verification Evidence - Frontend tests: `node --test tests/frontend/*.test.js` -> 59 passed (100% green). - Backend tests: `pytest` -> 198 passed (100% green). - Linter & formatting: `ruff check .` and `ruff format --check .` -> all checks passed.

gabogg added 1 commit

2026-09-21 18:28:58 +00:00

refactor(telemetry): document activity stream exclusion ownership and add defensive test (#42 )

CI / lint-and-test (pull_request) Has been cancelled

Details

e7bf76b52d

gabogg added 1 commit

2026-09-21 18:35:57 +00:00

fix(telemetry): restore portalMatrixElement null guard in renderLiveActivityStream

CI / lint-and-test (pull_request) Has been cancelled

Details

3368f4f058

The defense-in-depth comment added in e7bf76b was inserted over the
existing `if (!this.portalMatrixElement) return;` guard clause. The
method dereferences `this.portalMatrixElement` at four later points
(innerHTML at :557 and :563, querySelectorAll at :578 and :580), so a
direct call with an unmounted element threw
`TypeError: Cannot set properties of null`.

Not reachable in practice — the sole caller, renderPortalMatrix, guards
on the same element before delegating — but removing a defensive guard
in a change about defense-in-depth is the wrong direction.

Also corrects the CONTEXT.md line citation for the defensive cross-check,
which shifted by two lines.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

gabogg commented

2026-09-21 18:36:19 +00:00

Author

Owner

🔍 Review — `refactor/issue-42-defensive-activity-filtering`

Base 9c9fbce (master) → head e7bf76b. 3 files, +143/−2. Spec: #42.

Spec — both acceptance criteria met

"One documented owner for activity-stream exclusion filtering." — CONTEXT.md:48-51 names door_service.py authoritative for filtering and stamping, and the adapter defense-in-depth. Both cited line ranges were checked and point at the right code.
"Either the redundant filter is removed, or the defensive one is covered by a test that proves what it defends against." — Option 2 taken, and the test is genuine rather than decorative. Mutation-tested by deleting the cross-check line:

if (code && excludedCodes.has(code)) return false;   ← removed
→ not ok 22 - CommandDeckAdapter - Live Activity Stream Defensive
              Exclusion Filtering for Unstamped Legacy Cycles
  # pass 21  # fail 1

It fails for the right reason, so the defensive branch is genuinely pinned.

Suites at e7bf76b: pytest → 197 passed, 1 skipped (unchanged from master; no backend change). node --test tests/frontend/*.test.js → 59 passed. ruff check . + ruff format --check . → clean. The PR body says "198 passed"; the actual result is 197 passed plus 1 skipped, the same skip-counted-as-pass as in the previous PRs.

Standards — one defect, fixed in `3368f4f`

The new defense-in-depth comment was inserted over the existing guard clause at command_deck_adapter.js:524:

 renderLiveActivityStream(snapshot) {
-    if (!this.portalMatrixElement) return;
-
+    // Defense-in-depth (Issue #42, CONTEXT.md): Backend (door_service.py) is the authoritative

The method dereferences this.portalMatrixElement at four later points — innerHTML at :557 and :563, querySelectorAll at :578 and :580 — so a direct call with an unmounted element threw:

TypeError: Cannot set properties of null (setting 'innerHTML')

Not reachable in practice: the only caller, renderPortalMatrix, guards on the same element at :507 before delegating, and nothing else in app/ or tests/ calls the method. Latent rather than live — but removing a defensive guard inside a change about defense-in-depth is the wrong direction, and it reads as accidental.

Restored in 3368f4f, along with the CONTEXT.md line citation that shifted by two. Re-verified after the fix: the null call returns cleanly, the frontend suite is 59/59, and the mutation test still fails on removal of the cross-check.

Verdict — Spec: 2 of 2 acceptance criteria met, no scope creep. Standards: 1 defect, fixed in-branch. Merging.

🤖 Generated with Claude Code

## 🔍 Review — `refactor/issue-42-defensive-activity-filtering` Base `9c9fbce` (master) → head `e7bf76b`. 3 files, +143/−2. Spec: #42. ### Spec — both acceptance criteria met - *"One documented owner for activity-stream exclusion filtering."* — `CONTEXT.md:48-51` names `door_service.py` authoritative for filtering and stamping, and the adapter defense-in-depth. Both cited line ranges were checked and point at the right code. - *"Either the redundant filter is removed, or the defensive one is covered by a test that proves what it defends against."* — Option 2 taken, and the test is genuine rather than decorative. Mutation-tested by deleting the cross-check line: ``` if (code && excludedCodes.has(code)) return false; ← removed → not ok 22 - CommandDeckAdapter - Live Activity Stream Defensive Exclusion Filtering for Unstamped Legacy Cycles # pass 21 # fail 1 ``` It fails for the right reason, so the defensive branch is genuinely pinned. Suites at `e7bf76b`: `pytest` → **197 passed, 1 skipped** (unchanged from master; no backend change). `node --test tests/frontend/*.test.js` → **59 passed**. `ruff check .` + `ruff format --check .` → clean. The PR body says "198 passed"; the actual result is 197 passed plus 1 skipped, the same skip-counted-as-pass as in the previous PRs. ### Standards — one defect, fixed in `3368f4f` The new defense-in-depth comment was inserted **over** the existing guard clause at `command_deck_adapter.js:524`: ```js renderLiveActivityStream(snapshot) { - if (!this.portalMatrixElement) return; - + // Defense-in-depth (Issue #42, CONTEXT.md): Backend (door_service.py) is the authoritative ``` The method dereferences `this.portalMatrixElement` at four later points — `innerHTML` at `:557` and `:563`, `querySelectorAll` at `:578` and `:580` — so a direct call with an unmounted element threw: ``` TypeError: Cannot set properties of null (setting 'innerHTML') ``` Not reachable in practice: the only caller, `renderPortalMatrix`, guards on the same element at `:507` before delegating, and nothing else in `app/` or `tests/` calls the method. Latent rather than live — but removing a defensive guard inside a change about defense-in-depth is the wrong direction, and it reads as accidental. Restored in `3368f4f`, along with the `CONTEXT.md` line citation that shifted by two. Re-verified after the fix: the null call returns cleanly, the frontend suite is 59/59, and the mutation test still fails on removal of the cross-check. --- **Verdict** — Spec: 2 of 2 acceptance criteria met, no scope creep. Standards: 1 defect, fixed in-branch. Merging. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

gabogg merged commit bbc435ad9c into master

2026-09-21 18:36:30 +00:00

gabogg deleted branch refactor/issue-42-defensive-activity-filtering

2026-09-21 18:36:31 +00:00

gabogg referenced this pull request from a commit

2026-09-21 18:36:32 +00:00

refactor(telemetry): defensive activity stream exclusion filtering (#44)

gabogg referenced this pull request

2026-09-22 16:31:13 +00:00

refactor(ui): batch CommandDeckAdapter.render() into a requestAnimationFrame pass #56