Merge pull request #1785 from netalertx/next_release

Next release

.claude/skills/database-patterns/SKILL.md

@@ -9,7 +9,7 @@ description: Read before designing a feature that writes to the Devices table, a
 
 Before implementing any feature that reads or writes the `Devices` table, audit ALL write paths. The table is modified from many locations — missing one path is a correctness bug.
 
-**Known production write paths (as of 2026-07-04):**
+**Known production write paths:**
 
 | File | Function | Fields written |
 |---|---|---|

.claude/skills/plugin-development/SKILL.md

@@ -33,7 +33,8 @@ server/plugins/<code_name>/
 - `<PREF>_CMD`: script path.
 - `<PREF>_RUN_TIMEOUT`: timeout in seconds — **enforced by the core plugin runner as the whole script's kill-timeout** (`server/plugin.py` passes it straight to `subprocess(..., timeout=...)`). Not a safe per-HTTP-call timeout — don't reuse it for individual network calls in a loop, or one slow call can burn the whole budget and get the process killed before it writes its result file. Two correct alternatives: `config.json`'s `"timeoutMultiplier": true` on a `params[]` entry for a config-declared, known-length loop (see `arp_scan`); `plugin_helper.per_item_timeout()` for a runtime-variable-length loop (see the `_publisher_*` plugins).
 - `<PREF>_WATCH`: columns to watch for changes.
-- `<PREF>_IMPORT_ON`: optional — gates whether this run's rows get promoted into `CurrentScan` (only relevant if `mapped_to_table: "CurrentScan"`). See `docs/PLUGINS_DEV_DATA_CONTRACT.md` for the related per-row `scanCreatesDevice`/`scanNotificationMode`/`scanPresence` columns.
+- `<PREF>_IMPORT_ON`: optional — gates whether this run's rows get promoted into `CurrentScan` (only relevant if `mapped_to_table: "CurrentScan"`). See `docs/PLUGINS_IMPORT_BEHAVIOR.md` for the related per-row `scanCreatesDevice`/`scanNotificationMode`/`scanPresence` columns.
+- **`dataType` and `default_value` must agree.** `dataType: "array"`/`"object"` needs a real JSON literal for `default_value` (`'["default"]'`), not a bare string (`"default"`). `setting_value_to_python_type()` (`server/helper.py`) `json.loads()`s the default at runtime; a bare string fails silently — logged, and `[]` is returned instead of your default (e.g. `devParentRelType`, `UI_theme`, `UI_TOPOLOGY_ORDER`). If `elementOptions` already sets `multiple`/`orderable: "false"`, the setting is scalar — use `dataType: "string"` instead.
 
 ## Data Contract
 
@@ -70,7 +71,7 @@ Full column spec: `docs/PLUGINS_DEV_DATA_CONTRACT.md`. Note `helpVal1-4`/`watche
 
 ## Before Opening a PR
 
-Check the plugin against the [Conventions Checklist](../../../docs/PLUGINS_DEV.md#conventions-checklist) — `RUN` default, schedule precedent, `RUN_TIMEOUT` semantics, reusing core settings instead of duplicating them, description length (renders in the Settings UI — keep it short), and the multi-instance settings pattern (nested array + popup-form, see `rest_import`, not a hardcoded "primary"/"secondary" pair). Most plugin PR review comments trace back to one of these, and `test/plugins/test_plugin_conventions.py` mechanically enforces the RUN-default, description-length, hardcoded-default-drift, and RUN_TIMEOUT-reuse-in-loop items — run it after touching a plugin.
+Check the plugin against the [Conventions Checklist](../../../docs/PLUGINS_DEV.md#conventions-checklist) — `RUN` default, schedule precedent, `RUN_TIMEOUT` semantics, reusing core settings instead of duplicating them, description length (renders in the Settings UI — keep it short), and the multi-instance settings pattern (nested array + popup-form, see `rest_import`, not a hardcoded "primary"/"secondary" pair). Most plugin PR review comments trace back to one of these, and `test/plugins/test_plugin_conventions.py` mechanically enforces the RUN-default, description-length, hardcoded-default-drift, RUN_TIMEOUT-reuse-in-loop, and array/object dataType-default_value-mismatch items — run it after touching a plugin.
 
 ## Starting Point
 

.claude/skills/plugin-readme/SKILL.md

@@ -21,7 +21,7 @@ Exception: call out a *specific* setting by name, in prose, only when its behavi
 
 ## Verify against the actual code first
 
-Read `config.json` (`unique_prefix`, `plugin_type`, `data_source`, `settings`) and the plugin's script before writing anything - don't guess at mechanism from the plugin's name alone. Real bugs found this way during a past audit: `dig_scan/README.md` described the `nbtscan` utility (copy-paste from a sibling plugin); `adguard_import/README.md` was a byte-for-byte copy of `__template/README.md`, never actually written.
+Read `config.json` (`unique_prefix`, `plugin_type`, `data_source`, `settings`) and the plugin's script before writing anything - don't guess at mechanism from the plugin's name alone. A README copied from a sibling plugin or left as the unedited `__template/README.md` describes the wrong plugin's behavior.
 
 ## Backfilling missing "Other info"
 

.claude/skills/prd-writing/SKILL.md

@@ -11,12 +11,12 @@ Triggered by: "write a PRD", "draft a design doc", "spec out this feature", "cre
 
 ## Core principle: a PRD is a claim-verification exercise, not a writing exercise
 
-Every sentence that asserts something about how the code currently works must be checked against the actual code before it goes in — not written from memory, not inferred from a plugin's name or reputation, not assumed because it sounds plausible. Two real, caught-in-review examples from this exact process:
+Every sentence that asserts something about how the code currently works must be checked against the actual code before it goes in — not written from memory, not inferred from a plugin's name or reputation, not assumed because it sounds plausible. Two failure patterns to watch for:
 
-- A draft claimed "plugin X's rows are the cleanest case for a static presence-flag value" — reasonable-sounding, and wrong. Reading the actual script showed it already reports a live per-row state field and computes an equivalent boolean internally that was simply never wired up. The claim was never checked against the script, only against the plugin's category ("reservation-style").
-- A draft claimed "no changes needed here" for two queries when adding a new presence column, on the reasoning that they were already "inert" for the new case. True for one sub-case (a device that starts absent), false for the transition sub-case (a device going from online to newly-suppressed) — because those two queries used a different existence check than the one already patched. It was missed for a full turn, until a direct question ("does this handle the transition where a device *was* online?") forced a re-trace of the actual call graph.
+- Calling a plugin "the cleanest case" for some behavior based on its category alone. Read the actual script — it may already report a live per-row signal and compute an equivalent value internally that was never wired up.
+- Claiming "no changes needed" for a query based on one sub-case (e.g. a device that starts absent) without checking the transition sub-case (a device going from online to newly-suppressed) — the two sub-cases can use different existence checks, so a fix covering one can silently miss the other.
 
-Both mistakes were plausible, well-written, and wrong. Neither would have survived actually reading the code first.
+Both are plausible, well-written, and wrong. Reading the code first catches both.
 
 ## Process
 
@@ -28,7 +28,7 @@ Both mistakes were plausible, well-written, and wrong. Neither would have surviv
 6. **Force every open question to an explicit decision**, even if the decision is "accept as-is for v1, revisit if feedback says otherwise." An open question left unresolved in a PRD gets silently decided by whoever implements it — usually differently than anyone actually intended.
 7. **Write the test plan as part of the PRD, not after.** Concrete test cases — naming real functions/queries, not "add tests for X" — force you to notice design gaps you'd otherwise miss; the moment you try to write "assert Y happens" and realize the current design can't produce Y is often the first time the gap becomes visible. Check the repo for an existing test pattern for this shape of change before inventing a new one (e.g. a prior presence-logic bug fixed via `test/db_test_helpers.py` fixtures is the template for the next one, not a reason to build new test infrastructure).
 8. **Ask explicitly whether validating this needs real end-to-end infrastructure** (a new or modified plugin, a UI click-through) or whether synthetic unit-level fixtures suffice — don't assume either way. Check whether the functions under test take a DB connection/dict/list as a parameter (testable in isolation, no real plugin needed) or require a real file on disk (harder to fake, may need one).
-9. **Evaluate performance impact against the schema that actually exists, not the schema you'd expect, and against real deployment scale, not an imagined one.** For every new or modified query: does it reuse an existing index, or does it add an unindexed lookup, a new join, or a correlated subquery? Check for real — don't assume a column is indexed just because it looks identity-like (`CurrentScan.scanMac` looked like exactly the kind of column that should have an index; grepping for `CREATE INDEX` showed this codebase never gave it one, and neither did the first draft of the design that needed it — since fixed, `idx_currentscan_scanmac` now exists in `server/db/db_upgrade.py:ensure_CurrentScan()`, so check whether a later PRD's problem is already mitigated before assuming it's new). Then multiply the per-query cost by two things: how often it runs (a full scan inside a loop that fires once is nothing; the same scan inside a cycle that reruns every few minutes forever is a standing cost, permanently), and the actual scale this project runs at — **known real production users run 10,000+ devices** (confirmed directly by the project owner, not a guess or an inference from `CLAUDE.md`'s "homelabs, MSPs, and NOCs" framing). At that scale, a `CurrentScan` populated at 2-5 rows per device (one per contributing plugin, the normal case) is routinely 20,000-50,000+ rows in a single cycle — treat that as the number to reason about, not a hypothetical upper bound reserved for some future large deployment. Concrete example from this process: implementing a new multi-source precedence rule as a correlated `EXISTS` subquery re-evaluated per candidate row reads as perfectly reasonable, passes every test at small scale, and is an accidental self-join with no index behind it at scale — the fix (add the missing index, express the aggregation as one `GROUP BY` pass instead of a per-row correlated check) had to be written into the PRD explicitly, or it would have shipped as a footgun that real 10k-device users would have hit, not a theoretical one.
+9. **Check performance against the real schema and real scale, not assumptions.** For every new or changed query: does it use an existing index, or add an unindexed lookup, a new join, or a correlated subquery? Grep `CREATE INDEX` — don't assume a column is indexed just because it looks like a key (check `server/db/db_upgrade.py`/`server/db/schema/app.sql`). Then weigh cost by how often the query runs (once is nothing; every few minutes forever is a standing cost) and by real scale — **production users run 10,000+ devices**, not a homelab handful. A `CurrentScan` with 2-5 rows per device (one per contributing plugin) is routinely 20,000-50,000+ rows in one cycle; reason about that number, not a smaller hopeful one. A correlated `EXISTS`/subquery re-evaluated per outer row is fine *if* the correlated column is indexed — e.g. `current_scan_presence_condition()` (`server/scan/presence.py`) is exactly this shape against `CurrentScan.scanMac`, covered by `idx_currentscan_scanmac`. The real risk is an unindexed correlated lookup: an accidental self-join scanning the full inner table per outer row, which looks fine and passes tests at small scale but isn't at production scale. Check with `EXPLAIN QUERY PLAN` at a realistic row count rather than assuming either way; if it comes back unindexed, a `GROUP BY` aggregate is the usual fix.
 10. **Do a dedicated final-check pass, out loud, before calling it done.** Re-read the whole document end to end and specifically check:
     - Did a correction made mid-document actually propagate everywhere it needed to (the Design subsection *and* Affected Files *and* Tests *and* any execution-plan summary)? A correction landing in one place and not its siblings is worse than never catching it, because now the document silently contradicts itself.
     - Does every "this is the cleanest/simplest real case" claim still hold up if you actually re-read that specific piece of code right now, or was it asserted by pattern-matching a name/category? Re-verify, don't re-assert.

.claude/skills/scan-pipeline/SKILL.md

@@ -7,57 +7,57 @@ description: Read before modifying the scan pipeline (server/scan/session_events
 
 ## Scope
 
-This skill covers what happens *after* a plugin's rows land in `CurrentScan` — presence computation, event generation, and session/timeline derivation. It does not cover the plugin-authoring side (manifest, data contract, settings) — see the `plugin-development` skill and `docs/PLUGINS_DEV*.md` for that. It also does not cover the general `*Source` attribution system or SQLite triggers for audit logging — see the `database-patterns` skill (Copilot tree only, as of this writing) for that; the field-authority section below is the scan-pipeline-local half of that bigger system, and the two overlap.
+Covers what happens after a plugin's rows land in `CurrentScan`: presence computation, event generation, session/timeline derivation. Not plugin authoring (manifest, data contract, settings) — see `plugin-development` and `docs/PLUGINS_DEV*.md`. Not the general `*Source` attribution system or SQLite audit triggers — see `database-patterns`; the field-authority section below is the scan-pipeline-local half of that system.
 
-## The core tables/views and their lifetimes
+## Core tables and views
 
-- **`CurrentScan`** — ephemeral scratch table. Populated by `process_plugin_events()` for any plugin whose `config.json` declares `mapped_to_table`, then fully deleted at the end of every `process_scan()` cycle (`DELETE FROM CurrentScan`). Never assume a value written into a `CurrentScan` row is readable outside the cycle it arrived in — by the time a later cycle needs to check "was this row flagged X," the row is gone. This bit a real design (see Gotcha 2 below).
+- **`CurrentScan`** — ephemeral scratch table. `process_plugin_events()` populates it for any plugin whose `config.json` declares `mapped_to_table`; `process_scan()` deletes all rows at the end of every cycle. A value written to a `CurrentScan` row is not readable in a later cycle — the row is gone by then. See Gotcha 2.
 - **`Devices`** — persistent identity + state table.
-- **`Events`** — persistent, append-only log of state-transition events (`New Device`, `Connected`, `Down Reconnected`, `Device Down`, `Disconnected`, `IP Changed`). This *is* the audit trail; `Sessions` is derived from it, not the other way around.
-- **`Sessions`** — not incrementally updated; fully wiped and rebuilt every cycle from a view (see `Convert_Events_to_Sessions` below). Don't reason about it as a live connection state machine — it's a materialized query result recomputed from `Events` each cycle.
-- **`Online_History`** — persistent, one row per scan cycle, feeds the dashboard's online/offline graph. Purely a rollup of `devPresentLastScan`/`devAlertDown`/`devIsSleeping` counts on `DevicesView` — no independent state of its own.
+- **`Events`** — persistent, append-only log of state-transition events (`New Device`, `Connected`, `Down Reconnected`, `Device Down`, `Disconnected`, `IP Changed`). This is the audit trail; `Sessions` is derived from it, not the reverse.
+- **`Sessions`** — fully wiped and rebuilt every cycle from `Convert_Events_to_Sessions` (below), not incrementally updated. Treat it as a materialized query result, not a live connection state machine.
+- **`Online_History`** — one row per scan cycle, feeds the dashboard's online/offline graph. A rollup of `devPresentLastScan`/`devAlertDown`/`devIsSleeping` counts on `DevicesView` — no state of its own.
 
 ## Key views
 
-- **`LatestDeviceScan`** (`server/db/db_upgrade.py`) — `Devices` LEFT JOIN'd to the most recent `CurrentScan` row **per `(scanMac, scanSourcePlugin)` pair**, ranked via `ROW_NUMBER() OVER (PARTITION BY scanMac, scanSourcePlugin ...)`. This is why `update_devices_data_from_scan()` loops over `DISTINCT scanSourcePlugin` and re-queries this view once per plugin: when two plugins report the same device in the same cycle, they are *not* merged into one row before processing — each plugin's contribution is evaluated independently, per field, through the authority mechanism below.
-- **`LatestEventsPerMAC`** — most recent Event per MAC, joined to `Devices` and `CurrentScan`. Used by the "New Connections" query in `insert_events()` to decide whether a device was previously down (→ `Down Reconnected`) or genuinely new (→ `Connected`).
-- **`Convert_Events_to_Sessions`** — the actual definition of "is this device's session still open." **There is no `close_session()`-style function anywhere in this codebase.** A session closes purely as an emergent property: `pair_sessions_events()` sets `evePairEventRowid` on a `New Device`/`Connected`/`Down Reconnected` Event to point at the next `Disconnected`/`Device Down` Event for that MAC, and this view computes `sesStillConnected = 1` exactly when that pairing is still `NULL`. If a session needs to close, the fix is always "make sure the right `Events` row gets inserted" — never a direct `Sessions` mutation (the one exception is `create_new_devices()`'s reconnect-insert, noted below).
-- **`DevicesView`** — adds computed `devIsSleeping`/`devFlapping`/`devStatus` on top of `Devices`. This is what the UI and `insertOnlineHistory()` actually read presence from, not the raw `Devices` table.
+- **`LatestDeviceScan`** (`server/db/db_upgrade.py`) — `Devices` LEFT JOIN'd to the most recent `CurrentScan` row per `(scanMac, scanSourcePlugin)` pair, via `ROW_NUMBER() OVER (PARTITION BY scanMac, scanSourcePlugin ...)`. `update_devices_data_from_scan()` loops over `DISTINCT scanSourcePlugin` and re-queries this view once per plugin: when two plugins report the same device in one cycle, each contribution is evaluated separately, per field, through the authority mechanism below — they are not merged into one row first.
+- **`LatestEventsPerMAC`** — most recent Event per MAC, joined to `Devices` and `CurrentScan`. The "New Connections" query in `insert_events()` uses it to decide whether a device was previously down (→ `Down Reconnected`) or new (→ `Connected`).
+- **`Convert_Events_to_Sessions`** — defines "is this device's session still open." There is no `close_session()` function anywhere in this codebase. A session closes as an emergent property: `pair_sessions_events()` sets `evePairEventRowid` on a `New Device`/`Connected`/`Down Reconnected` Event to point at the next `Disconnected`/`Device Down` Event for that MAC; this view sets `sesStillConnected = 1` exactly when that pairing is `NULL`. To close a session, insert the right `Events` row — never mutate `Sessions` directly (the one exception is `create_new_devices()`'s reconnect-insert, in the call order below).
+- **`DevicesView`** — adds computed `devIsSleeping`/`devFlapping`/`devStatus` on top of `Devices`. The UI and `insertOnlineHistory()` read presence from this, not the raw `Devices` table.
 
-## `process_scan()` call order (`server/scan/session_events.py`) — the order is load-bearing, not incidental
+## `process_scan()` call order (`server/scan/session_events.py`) — order is load-bearing
 
 1. `save_own_device()`, `exclude_ignored_devices()`
-2. `insert_events(db)` — **runs before presence gets updated for this cycle.** Deliberate: the Down/Disconnected/Connected queries need the *previous* cycle's `devPresentLastScan` value to detect a transition (present last cycle but absent now, or vice versa). If this ran after the presence update, every query would see the already-updated value and the edge-triggered design would collapse into either never firing or firing every cycle.
-3. `create_new_devices(db)` — the source comment is explicit: "after create events -> avoid 'connection' event." Brand-new devices get a `New Device` event instead of a `Connected` event, because at step 2 they didn't exist as `Devices` rows for either query to match. Also contains a raw `INSERT INTO Sessions ... sesStillConnected = 1` for devices that already exist but have no currently-open session — the one place outside the `Events`-derived path that writes `Sessions` directly.
+2. `insert_events(db)` — runs before presence updates for this cycle. The Down/Disconnected/Connected queries need the *previous* cycle's `devPresentLastScan` to detect a transition. If this ran after the presence update, every query would see the new value and the edge-triggered design would break — firing never, or every cycle.
+3. `create_new_devices(db)` — runs before presence updates so a brand-new device gets a `New Device` event, not a `Connected` event (it has no `Devices` row yet for step 2's queries to match). Also has a raw `INSERT INTO Sessions ... sesStillConnected = 1` for existing devices with no open session — the one place outside the `Events`-derived path that writes `Sessions` directly.
 4. `update_devices_data_from_scan(db)` — field-level updates for existing devices; see the authority mechanism below.
 5. `update_sync_hub_node`, `update_devLastConnection_from_CurrentScan`
-6. `update_presence_from_CurrentScan(db)` — sets `devPresentLastScan` from bare `CurrentScan` presence *for this cycle* (this becomes the "previous" value step 2 reads on the *next* cycle).
-7. `update_devPresentLastScan_based_on_nics(db)` — NIC/parent-child presence aggregation; can override step 6's result for parent devices.
-8. `update_devPresentLastScan_based_on_force_status(db)` — the user's manual `devForceStatus` override, runs **last**, wins unconditionally over everything above.
+6. `update_presence_from_CurrentScan(db)` — sets `devPresentLastScan` from `CurrentScan` for this cycle (step 2 reads this as "previous" on the *next* cycle).
+7. `update_devPresentLastScan_based_on_nics(db)` — NIC/parent-child presence aggregation; can override step 6 for parent devices.
+8. `update_devPresentLastScan_based_on_force_status(db)` — the user's manual `devForceStatus` override; runs last, wins over everything above.
 9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types`
 10. `pair_sessions_events(db)` — pairs `Events` rows as described above.
-11. `create_sessions_snapshot(db)` — `DELETE FROM Sessions; INSERT INTO Sessions SELECT * FROM Convert_Events_to_Sessions`. This is the point where `Sessions` actually reflects step 10's pairing.
+11. `create_sessions_snapshot(db)` — `DELETE FROM Sessions; INSERT INTO Sessions SELECT * FROM Convert_Events_to_Sessions`. `Sessions` reflects step 10's pairing from here.
 12. `insertOnlineHistory(db)` — dashboard graph rollup.
 13. `skip_repeated_notifications(db)`
-14. `DELETE FROM CurrentScan` — the ephemeral table's entire lifetime is one call to `process_scan()`.
+14. `DELETE FROM CurrentScan` — the table's entire lifetime is one call to `process_scan()`.
 
 ## Field-write authority for scan-derived updates
 
-`update_devices_data_from_scan()` (`server/scan/device_handling.py`) does not blindly overwrite fields from whichever plugin ran most recently. Each trackable field is declared once in `FIELD_SPECS` (`scan_col`, `source_col`, a `priority` list of plugin prefixes, optional `allow_override_if_changed`), and `can_overwrite_field()` uses that plus `get_plugin_authoritative_settings()` (which reads a plugin's own settings for an explicit authority override) to decide, per field per row, whether this plugin's value may replace what's there. The paired `<field>Source` column (`devNameSource`, `devLastIPSource`, etc.) records who currently owns the field. `devMac` itself is never a target of these updates — it's the join key, not a tracked field — so no scan-derived update path can ever alter a device's identity, only its attributes.
+`update_devices_data_from_scan()` (`server/scan/device_handling.py`) does not overwrite fields from whichever plugin ran most recently. Each trackable field is declared once in `FIELD_SPECS` (`scan_col`, `source_col`, a `priority` list of plugin prefixes, optional `allow_override_if_changed`). `can_overwrite_field()` uses that plus `get_plugin_authoritative_settings()` (a plugin's own authority-override setting, if any) to decide, per field per row, whether this plugin's value may replace what's there. The paired `<field>Source` column (`devNameSource`, `devLastIPSource`, etc.) records who currently owns the field. `devMac` is never a target of these updates — it's the join key, not a tracked field — so no scan-derived update can alter a device's identity, only its attributes.
 
-This is the scan-pipeline-local half of a bigger attribution system — see the `database-patterns` skill for `FIELD_SOURCE_MAP` / `server/db/authoritative_handler.py`, the full `*Source` attribution model, and how SQLite triggers consume it for audit logging. Read both if touching anything that writes a `*Source` column.
+This is the scan-pipeline-local half of a bigger attribution system — see `database-patterns` for `FIELD_SOURCE_MAP`/`server/db/authoritative_handler.py`, the full `*Source` model, and the SQLite triggers that consume it for audit logging. Read both before touching anything that writes a `*Source` column.
 
-## Four real gotchas (not hypothetical — all surfaced live during a design review)
+## Gotchas
 
-1. **A "presence" check almost always exists in more than one place.** When adding a per-row signal meaning "don't count this as a live sighting" (e.g. a proposed `scanPresence` column), every query that independently re-derives "is this MAC currently present" from `CurrentScan` has to be updated together — `update_presence_from_CurrentScan()`, the `insert_events()` "New Connections"/"Device Down"/"Disconnected" queries, and the raw `INSERT INTO Sessions` inside `create_new_devices()` all encode that same question separately. Patching one and missing a sibling produces a UI where the device badge, the Events log, and the Sessions timeline each tell a different story for the same device. See `.gemini/internal-docs/PRDs/plugin-import-behavior-controls.md` for the worked example — a `scanPresence = 0` transition that never closed its session because only one of three "is it present" queries had been patched. **Update:** `current_scan_presence_condition()` (`server/scan/presence.py`) now centralizes this for five of those sites — `update_presence_from_CurrentScan()` (both statements), `update_devLastConnection_from_CurrentScan()`, and three of `insert_events()`'s four queries (both `Device Down` variants, `Disconnected`) all call it instead of writing their own `EXISTS (...)`. The remaining two ("New Connections", the raw `Sessions` insert in `create_new_devices()`) still can't use it — they need the actual `scanLastIP`/`scanVendor` *value* off the presence-asserting row via `MIN()`/`GROUP BY`, not just a boolean — so a brand-new presence-adjacent query still has to be checked against both patterns, not assumed to be a bare helper call.
-2. **`CurrentScan` is deleted at the end of every cycle — a per-row flag on it cannot express a decision that needs to survive to a cycle where the row is absent.** Anything that fires specifically *because* a row is missing (`Device Down`, `Disconnected`) cannot read a flag that lived on that now-gone row. If a per-row plugin signal needs to affect behavior beyond the cycle it arrived in, persist it onto the `Devices` row at creation time (e.g. seeding `devAlertDown`/`devAlertEvents` from the row's flag instead of the global `NEWDEV_*` defaults) rather than trying to make the ephemeral table carry it forward.
-3. **`CurrentScan` is not small, and it has an index now — check before assuming otherwise.** Real production users run 10,000+ devices; with the normal one-row-per-contributing-plugin pattern (see `LatestDeviceScan` above), a single cycle's `CurrentScan` is routinely 20,000-50,000+ rows, not the few hundred a homelab install might suggest. `idx_currentscan_scanmac` was added to `server/db/db_upgrade.py:ensure_CurrentScan()` (and mirrored in `server/db/schema/app.sql`) specifically because every `scanMac`-keyed lookup in this file was a full table scan without it — confirmed via `EXPLAIN QUERY PLAN` before the fix. Note `ensure_CurrentScan()` itself (the `DROP TABLE`/`CREATE TABLE`) only runs once, at app startup (`DB.initDB()`, called once from `server/__main__.py`) — don't confuse this with the per-cycle `DELETE FROM CurrentScan` in point 1 above, which clears rows but doesn't touch the table or its index. The index is built once and maintained incrementally, not rebuilt every cycle.
-4. **`server/plugins/sync/sync.py` bypasses this entire pipeline on purpose, twice — a permanent exception, not a bug.** It fires its own direct `INSERT OR IGNORE INTO Events (... 'New Device' ...)` for newly-seen synced devices (hardcoded `evePendingAlertEmail = 1`, no `scanNotificationMode`/quiet awareness), and in `carbon-copy` mode its own raw `Devices` UPSERT via `ON CONFLICT(devMac) DO UPDATE` — both deliberately skipping `create_new_devices()`/`update_devices_data_from_scan()`/`can_overwrite_field()` (`sync.py`'s own comments document this as intentional: "Node is fully authoritative in this mode"). It *is* a normal `mapped_to_table: CurrentScan` plugin for its presence contribution, so `IMPORT_ON`/`scanPresence` apply to it exactly like any other plugin — but its two direct-write paths would silently ignore `scanNotificationMode = 'quiet'` or `scanCreatesDevice = 0` if `sync` ever adopted either. Keep this in mind whenever touching the generic pipeline and assuming every `Events`/`Devices` write went through it — `sync.py` is the one place that doesn't.
-
-**Correction: `app.sql` is not dead code** — an earlier version of this note called it "otherwise-unused." Checked further: `install/production-filesystem/entrypoint.d/25-first-run-db.sh` pipes it straight into `sqlite3` to bootstrap a brand-new database on first install, and `scripts/db_cleanup/regenerate-database.sh` uses it too. `CurrentScan`, `Parameters`, and `Settings` are safe from drift because each has a dedicated `ensure_X()` function (`server/db/db_upgrade.py`) that unconditionally drops and recreates the table on every startup, superseding whatever `app.sql` bootstrapped. `Plugins_Language_Strings` gets the same unconditional drop/recreate, but inside the shared `ensure_plugins_tables()`, not a dedicated function of its own. `AppEvents` gets an equivalent drop/recreate too, via a different mechanism — `AppEvent_obj.__init__()` (`server/workflows/app_events.py`) drops and recreates it on every startup, independent of `db_upgrade.py`. `Devices` has no drop/recreate, but `server/database.py` has 18 explicit `ensure_column()` calls that backfill any column missing from an older `app.sql` snapshot on every startup. **`Events`, `Sessions`, and `Notifications`** — the three tables that genuinely had neither safety net — **now have the same backfill treatment**, per `scan-pipeline-hardening.md` Design §3 (implemented): `ensure_table_columns()` (`server/db/db_upgrade.py`), driven by one Python column-list constant per table (`server/db/schema_columns.py`) that's also diffed against `app.sql` in CI (`test/db/test_schema_drift_guard.py`), so drift between the two is caught rather than silently shipping. `AppEvents`/`Notifications` each also have a *second* schema-definition surface beyond `app.sql` worth knowing about — their own inline `CREATE TABLE IF NOT EXISTS` in `server/workflows/app_events.py`/`server/models/notification_instance.py` respectively — kept in sync via the same drift-check test. Any *new* query added here should still be checked with `EXPLAIN QUERY PLAN` at a realistic row count rather than assumed fine because it "looks like the existing queries" — several of those existing queries were themselves unindexed scans until this was caught. A correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable and be quadratic at this scale.
+1. **A "presence" check exists in more than one place.** A per-row signal meaning "don't count this as a live sighting" (e.g. `scanPresence`) has to reach every query that independently re-derives "is this MAC currently present" from `CurrentScan`. `current_scan_presence_condition()` (`server/scan/presence.py`) centralizes that check for five sites: `update_presence_from_CurrentScan()` (both statements), `update_devLastConnection_from_CurrentScan()`, and three of `insert_events()`'s four queries (both `Device Down` variants, `Disconnected`). Two sites can't use it: the "New Connections" query and the raw `Sessions` insert in `create_new_devices()` need the actual `scanLastIP`/`scanVendor` value off the presence-asserting row via `MIN()`/`GROUP BY`, not just a boolean. Check any new presence-adjacent query against both patterns — a bare helper call isn't always enough.
+2. **`CurrentScan` is deleted at the end of every cycle — a per-row flag on it can't express a decision that needs to survive to a cycle where the row is gone.** Anything that fires because a row is *missing* (`Device Down`, `Disconnected`) can't read a flag that lived on that row. A per-row plugin signal that needs to affect behavior beyond its own cycle has to persist onto the `Devices` row at creation time (e.g. seeding `devAlertDown`/`devAlertEvents` from the row's flag instead of the global `NEWDEV_*` defaults), not ride on the ephemeral table.
+3. **`CurrentScan` is not small, and it's indexed on `scanMac`.** Real production users run 10,000+ devices; with one row per contributing plugin (see `LatestDeviceScan` above), a single cycle's `CurrentScan` is routinely 20,000-50,000+ rows. `idx_currentscan_scanmac` (`server/db/db_upgrade.py:ensure_CurrentScan()`, mirrored in `server/db/schema/app.sql`) covers every `scanMac`-keyed lookup in this file. `ensure_CurrentScan()`'s `DROP TABLE`/`CREATE TABLE` runs once, at app startup (`DB.initDB()`, `server/__main__.py`) — don't confuse this with the per-cycle `DELETE FROM CurrentScan` in point 1, which clears rows but leaves the table and its index in place.
+4. **`server/plugins/sync/sync.py` bypasses this pipeline on purpose, twice — a permanent exception, not a bug.** It fires its own direct `INSERT OR IGNORE INTO Events (... 'New Device' ...)` for newly-seen synced devices (hardcoded `evePendingAlertEmail = 1`, no `scanNotificationMode` awareness), and in `carbon-copy` mode its own raw `Devices` UPSERT via `ON CONFLICT(devMac) DO UPDATE` — both skip `create_new_devices()`/`update_devices_data_from_scan()`/`can_overwrite_field()` (`sync.py`'s own comments: "Node is fully authoritative in this mode"). It's a normal `mapped_to_table: CurrentScan` plugin for its presence contribution, so `IMPORT_ON`/`scanPresence` apply to it like any other plugin — but its two direct-write paths ignore `scanNotificationMode = 'quiet'` or `scanCreatesDevice = 0`. Don't assume every `Events`/`Devices` write goes through the generic pipeline — `sync.py` doesn't.
+5. **A blank/null-equivalent `scanMac` can create a phantom `Devices` row.** `create_new_devices()`'s two creation-path queries filter `scanMac NOT IN (NULL_EQUIVALENTS_SQL)` (`server/scan/device_handling.py`, `const.NULL_EQUIVALENTS_SQL`) as a backstop, because `scanCreatesDevice` defaults to `1` — any plugin reporting a row with no real MAC, without setting `scanCreatesDevice = 0` itself, would otherwise create a `devMac = ''` device, and every other blank-MAC row from every other plugin would then silently write onto it. The filter doesn't replace `scanCreatesDevice = 0` as the correct thing for a plugin to set on such rows; it keeps a MAC-less row inert when some other plugin forgets to. Check any new creation-adjacent query against blank `scanMac` too.
+6. **`app.sql` is not dead code.** `install/production-filesystem/entrypoint.d/25-first-run-db.sh` pipes it into `sqlite3` to bootstrap a brand-new database on first install; `scripts/db_cleanup/regenerate-database.sh` uses it too. `CurrentScan`, `Parameters`, and `Settings` are safe from drift: each has a dedicated `ensure_X()` function (`server/db/db_upgrade.py`) that drops and recreates the table on every startup, superseding whatever `app.sql` bootstrapped. `Plugins_Language_Strings` gets the same treatment inside the shared `ensure_plugins_tables()`. `AppEvents` gets its own drop/recreate via `AppEvent_obj.__init__()` (`server/workflows/app_events.py`), independent of `db_upgrade.py`. `Devices` has no drop/recreate, but `server/database.py` has 18 explicit `ensure_column()` calls that backfill any column missing from an older `app.sql` snapshot on every startup. `Events`, `Sessions`, and `Notifications` get the same backfill via `ensure_table_columns()` (`server/db/db_upgrade.py`), driven by one Python column-list constant per table (`server/db/schema_columns.py`) that's diffed against `app.sql` in CI (`test/db/test_schema_drift_guard.py`). `AppEvents`/`Notifications` each also have a second schema-definition surface — their own inline `CREATE TABLE IF NOT EXISTS` in `server/workflows/app_events.py`/`server/models/notification_instance.py` — kept in sync by the same drift-check test. Check any new query here with `EXPLAIN QUERY PLAN` at a realistic row count rather than assuming it's fine because it resembles an existing one — a correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable while actually being quadratic at this scale.
 
 ## When to read this vs. other docs/skills
 
-- Writing or reviewing a plugin's `config.json`/data contract → `plugin-development` skill, `docs/PLUGINS_DEV*.md`. This skill is about what happens *after* a plugin's rows land in `CurrentScan`, not the plugin-authoring contract itself.
-- Devices-table write paths, `*Source` attribution, audit/history logging, SQLite triggers → `database-patterns` skill.
-- Actually implementing a change here → read the relevant function in `server/scan/session_events.py` / `server/scan/device_handling.py` directly before trusting this skill's line-number references; they're a map, not a guarantee, and will drift as the code moves.
+- Writing or reviewing a plugin's `config.json`/data contract → `plugin-development`, `docs/PLUGINS_DEV*.md`. This skill covers what happens *after* a plugin's rows land in `CurrentScan`, not the authoring contract.
+- Devices-table write paths, `*Source` attribution, audit/history logging, SQLite triggers → `database-patterns`.
+- Implementing a change here → read the actual function in `server/scan/session_events.py`/`server/scan/device_handling.py` first; this skill's line numbers are a map, not a guarantee, and drift as the code moves.

.claude/skills/skill-hygiene/SKILL.md

@@ -0,0 +1,44 @@
+---
+name: skill-hygiene
+description: Read before writing or editing any SKILL.md in this repo (.claude/.gemini/.github skills trees). Covers the two standing rules for skill prose - state current behavior only, and prefer plain, short wording - plus the grep sweep to run before calling a skill clean.
+---
+
+# Skill Hygiene
+
+## What a skill is, and isn't
+
+A skill is a live reference for how the system works *now*. It is not a lab notebook, a changelog, or a PRD. History, corrections, and discovery narratives belong in PRDs (`.gemini/internal-docs/PRDs/`, see `prd-writing`) or commit messages — never in a skill body. If a skill needs to change because the code changed, edit it in place; don't leave a trace of what it used to say.
+
+This is a different rule from `prd-writing`'s "leave a visible trail of corrections" — that rule is specifically for PRDs, where the trail is the point. A skill has no reader who benefits from seeing your editing history; it only has readers who need the current fact, stated plainly.
+
+## Rule 1: state current behavior only
+
+Cut anything that narrates the past instead of stating the present:
+
+| Smell | Why it's a problem | Fix |
+|---|---|---|
+| `"Correction: X — an earlier version of this note said Y"` | Narrates an edit, not a fact | Just state the current fact about X |
+| `"(as of 2026-07-04)"` | Hedges instead of committing to the fact | State it as true now; update the line when it stops being true |
+| `"This shipped for real in devParentRelType"` / `"...caught in review"` / `"...caught mid-review"` | Tells the story of finding a bug instead of the rule that prevents it | State the rule; use the real example as a plain parenthetical if it helps, without the origin story |
+| `"Two real examples from this exact process:"` / `"during a past audit"` | Frames the skill as a diary of one session | Turn the anecdote into a timeless illustration, or drop it |
+| `"X now does Y"` / `"X previously did Y"` / `"used to be"` / `"no longer"` when describing *the skill's own past text* | Describes the skill's edit history, not the system | Delete — say what's true now, full stop |
+
+`"no longer"` describing real *system* behavior (e.g. "there is no longer a retry loop here") is fine — that's a fact about the code, not about the skill. The test is: does this sentence describe the codebase, or does it describe a previous version of this document?
+
+## Rule 2: plain words, fewer words
+
+If a shorter or simpler phrasing says the same thing, use it. Cut qualifiers that don't change the meaning ("actually," "really," "genuinely," "in this exact process"). Prefer a plain verb over a nominalization. A dense skill with real information beats a padded one — trim narration and hedging before trimming facts.
+
+## Sweep before calling a skill clean
+
+Run this across `.claude/skills/`, `.gemini/skills/`, `.github/skills/` (or a single file being edited):
+
+```bash
+grep -rniE "as of 202|caught in review|caught mid-review|correction:|correction \(|shipped for real|previously|used to be|no longer|originally|was later|historically|in the past|distilled from|real mistake|it turned out|turns out|discovered that|during a past" .claude/skills/ .gemini/skills/ .github/skills/
+```
+
+Read every hit in context — some are legitimate (a rule instructing PRD authors to write correction trails, or "previously down" describing device state, are not violations). Fix the ones that narrate the skill's own history instead of the system's current behavior.
+
+## Keep the three trees in sync
+
+Most skills exist as three near-identical copies (`.claude/skills/<name>/SKILL.md`, `.gemini/skills/<name>/SKILL.md`, `.github/skills/<name>/SKILL.md` — see `.gemini/skills/skills-index/SKILL.md` for the pairing map). When a hygiene fix changes a skill's body, apply the same fix to all paired copies so they stay identical (frontmatter `name`/`description` may differ per tree's own convention; the body should not). `scripts/check_skill_pairs.py`'s `GROUPS` list only flags when some-but-not-all paired files changed in a diff — it doesn't check the bodies actually match, so a manual diff after editing is still worth it.

.claude/skills/ux-design-patterns/SKILL.md

@@ -0,0 +1,35 @@
+---
+name: ux-design-patterns
+description: Read before adding or changing any front/ UI element - a control, layout, button, or interaction pattern. Covers the don't-invent-new-UX-without-a-PRD rule and the priority order for design tradeoffs (existing behavior > intuitiveness > information density > usability > utility > uniqueness > industry practices > generic UI).
+---
+
+# UX / Frontend Design Patterns
+
+## Core principle: reuse before inventing
+
+Don't introduce new UX behavior or visual patterns unless a PRD explicitly calls for it. Before building any new UI element, search the existing frontend for a pattern that already solves this exact need, and reuse its markup/CSS/behavior instead of inventing a new one.
+
+Real, recent example: a presence-page Prev/Next pager was first built with custom `<button class="btn btn-xs">` elements floated in a `.box-header`. The DataTables pagination pattern (`dataTables_wrapper` / `dataTables_paginate` / `ul.pagination` / `li.paginate_button.previous|next`) already existed elsewhere in the app and does the exact same job. The custom version looked visually broken in the actual UI and had to be reimplemented using the existing pattern once that was caught in manual testing - reusing it also picked up dark-mode theming (`front/css/dark-patch.css`'s `.pagination li > a` / `.disabled` rules) for free, which the hand-rolled version didn't have. Grep first: e.g. `grep -rn "pagination\|paginate_button" front/` before adding a "previous/next" control of your own; the same applies to modals, filter inputs, badges, tooltips, tables - anything that already has an established shape somewhere in `front/`.
+
+## Priority order for design decisions
+
+When several options are all locally reasonable, resolve the choice in this order - highest wins on conflict:
+
+1. **Existing behavior** - what does this codebase already do for the same or a similar need? Copy it.
+2. **Intuitiveness** - will a user already familiar with the rest of the app understand this without being told?
+3. **Information density** - does it show what's needed without wasting space or hiding what matters?
+4. **Usability** - is it easy and low-friction to actually use (reachability, click count, error tolerance)?
+5. **Utility** - does it solve the real problem, not just resemble a solution?
+6. **Uniqueness** - is this the app's own distinct answer, used only where nothing generic fits well?
+7. **Industry practices** - conventions users bring in from other apps.
+8. **Generic UI** - a default/framework-provided look, used only when nothing above applies.
+
+This list exists to end debates quickly, not to be argued from the bottom up. The reason it's written down is that #1 is exactly the step that gets skipped under time pressure - checking it first is meant to be fast, not a detour.
+
+## Practical checklist before building a new UI element
+
+1. Grep `front/js/`, `front/css/`, `front/php/` for an existing implementation of the same interaction - a table, a pager, a filter box, a modal, a badge, a status indicator.
+2. If found, reuse its markup and CSS classes directly rather than writing new ones - matching classes inherit theming (dark mode, responsive breakpoints) a new hand-rolled version won't have.
+3. If nothing fits, check whether the PRD driving this change actually calls for new UX. If it doesn't, that's a signal to look harder for an existing pattern, not license to invent one.
+4. If a new pattern is genuinely warranted and the PRD says so, design it using the priority order above, and record the choice and why existing patterns didn't fit in the PRD - the next change will hit the same fork and shouldn't have to re-derive the answer.
+5. Verify visually in a real browser/devcontainer before calling it done. A change that "should work" per the markup isn't confirmed until it's actually rendered - matches the project's general "test the golden path in a browser" rule for frontend changes.

.gemini/skills/database-patterns/SKILL.md

@@ -9,7 +9,7 @@ description: NetAlertX database architecture patterns. Use this when designing f
 
 Before implementing any feature that reads or writes the `Devices` table, audit ALL write paths. The table is modified from many locations — missing one path is a correctness bug.
 
-**Known production write paths (as of 2026-07-04):**
+**Known production write paths:**
 
 | File | Function | Fields written |
 |---|---|---|

.gemini/skills/plugin-development/plugin-skill.md

@@ -43,7 +43,8 @@ server/plugins/<code_name>/
 - `<PREF>_CMD`: script path
 - `<PREF>_RUN_TIMEOUT`: timeout in seconds — **this is enforced by the core plugin runner as the whole script's kill-timeout** (`server/plugin.py` passes it straight to `subprocess(..., timeout=...)`). It is not a safe per-HTTP-call timeout — don't reuse it for individual network calls in a loop, or one slow call can burn the whole budget and get the process killed before it writes its result file.
 - `<PREF>_WATCH`: columns to watch for changes
-- `<PREF>_IMPORT_ON`: optional — gates whether this run's rows get promoted into `CurrentScan` (only relevant if `mapped_to_table: "CurrentScan"`). See `docs/PLUGINS_DEV_DATA_CONTRACT.md` for the related per-row `scanCreatesDevice`/`scanNotificationMode`/`scanPresence` columns.
+- `<PREF>_IMPORT_ON`: optional — gates whether this run's rows get promoted into `CurrentScan` (only relevant if `mapped_to_table: "CurrentScan"`). See `docs/PLUGINS_IMPORT_BEHAVIOR.md` for the related per-row `scanCreatesDevice`/`scanNotificationMode`/`scanPresence` columns.
+- **`dataType` and `default_value` must agree.** `dataType: "array"`/`"object"` needs a real JSON literal for `default_value` (`'["default"]'`), not a bare string (`"default"`). `setting_value_to_python_type()` (`server/helper.py`) `json.loads()`s the default at runtime; a bare string fails silently — logged, and `[]` is returned instead of your default (e.g. `devParentRelType`, `UI_theme`, `UI_TOPOLOGY_ORDER`). If `elementOptions` already sets `multiple`/`orderable: "false"`, the setting is scalar — use `dataType: "string"` instead.
 
 ## Data Contract
 

.gemini/skills/plugin-readme/plugin-readme-skill.md

@@ -21,7 +21,7 @@ Exception: call out a *specific* setting by name, in prose, only when its behavi
 
 ## Verify against the actual code first
 
-Read `config.json` (`unique_prefix`, `plugin_type`, `data_source`, `settings`) and the plugin's script before writing anything - don't guess at mechanism from the plugin's name alone. Real bugs found this way during a past audit: `dig_scan/README.md` described the `nbtscan` utility (copy-paste from a sibling plugin); `adguard_import/README.md` was a byte-for-byte copy of `__template/README.md`, never actually written.
+Read `config.json` (`unique_prefix`, `plugin_type`, `data_source`, `settings`) and the plugin's script before writing anything - don't guess at mechanism from the plugin's name alone. A README copied from a sibling plugin or left as the unedited `__template/README.md` describes the wrong plugin's behavior.
 
 ## Backfilling missing "Other info"
 

.gemini/skills/prd-writing/SKILL.md

@@ -11,12 +11,12 @@ Triggered by: "write a PRD", "draft a design doc", "spec out this feature", "cre
 
 ## Core principle: a PRD is a claim-verification exercise, not a writing exercise
 
-Every sentence that asserts something about how the code currently works must be checked against the actual code before it goes in — not written from memory, not inferred from a plugin's name or reputation, not assumed because it sounds plausible. Two real, caught-in-review examples from this exact process:
+Every sentence that asserts something about how the code currently works must be checked against the actual code before it goes in — not written from memory, not inferred from a plugin's name or reputation, not assumed because it sounds plausible. Two failure patterns to watch for:
 
-- A draft claimed "plugin X's rows are the cleanest case for a static presence-flag value" — reasonable-sounding, and wrong. Reading the actual script showed it already reports a live per-row state field and computes an equivalent boolean internally that was simply never wired up. The claim was never checked against the script, only against the plugin's category ("reservation-style").
-- A draft claimed "no changes needed here" for two queries when adding a new presence column, on the reasoning that they were already "inert" for the new case. True for one sub-case (a device that starts absent), false for the transition sub-case (a device going from online to newly-suppressed) — because those two queries used a different existence check than the one already patched. It was missed for a full turn, until a direct question ("does this handle the transition where a device *was* online?") forced a re-trace of the actual call graph.
+- Calling a plugin "the cleanest case" for some behavior based on its category alone. Read the actual script — it may already report a live per-row signal and compute an equivalent value internally that was never wired up.
+- Claiming "no changes needed" for a query based on one sub-case (e.g. a device that starts absent) without checking the transition sub-case (a device going from online to newly-suppressed) — the two sub-cases can use different existence checks, so a fix covering one can silently miss the other.
 
-Both mistakes were plausible, well-written, and wrong. Neither would have survived actually reading the code first.
+Both are plausible, well-written, and wrong. Reading the code first catches both.
 
 ## Process
 
@@ -28,7 +28,7 @@ Both mistakes were plausible, well-written, and wrong. Neither would have surviv
 6. **Force every open question to an explicit decision**, even if the decision is "accept as-is for v1, revisit if feedback says otherwise." An open question left unresolved in a PRD gets silently decided by whoever implements it — usually differently than anyone actually intended.
 7. **Write the test plan as part of the PRD, not after.** Concrete test cases — naming real functions/queries, not "add tests for X" — force you to notice design gaps you'd otherwise miss; the moment you try to write "assert Y happens" and realize the current design can't produce Y is often the first time the gap becomes visible. Check the repo for an existing test pattern for this shape of change before inventing a new one (e.g. a prior presence-logic bug fixed via `test/db_test_helpers.py` fixtures is the template for the next one, not a reason to build new test infrastructure).
 8. **Ask explicitly whether validating this needs real end-to-end infrastructure** (a new or modified plugin, a UI click-through) or whether synthetic unit-level fixtures suffice — don't assume either way. Check whether the functions under test take a DB connection/dict/list as a parameter (testable in isolation, no real plugin needed) or require a real file on disk (harder to fake, may need one).
-9. **Evaluate performance impact against the schema that actually exists, not the schema you'd expect, and against real deployment scale, not an imagined one.** For every new or modified query: does it reuse an existing index, or does it add an unindexed lookup, a new join, or a correlated subquery? Check for real — don't assume a column is indexed just because it looks identity-like (`CurrentScan.scanMac` looked like exactly the kind of column that should have an index; grepping for `CREATE INDEX` showed this codebase never gave it one, and neither did the first draft of the design that needed it — since fixed, `idx_currentscan_scanmac` now exists in `server/db/db_upgrade.py:ensure_CurrentScan()`, so check whether a later PRD's problem is already mitigated before assuming it's new). Then multiply the per-query cost by two things: how often it runs (a full scan inside a loop that fires once is nothing; the same scan inside a cycle that reruns every few minutes forever is a standing cost, permanently), and the actual scale this project runs at — **known real production users run 10,000+ devices** (confirmed directly by the project owner, not a guess or an inference from `CLAUDE.md`'s "homelabs, MSPs, and NOCs" framing). At that scale, a `CurrentScan` populated at 2-5 rows per device (one per contributing plugin, the normal case) is routinely 20,000-50,000+ rows in a single cycle — treat that as the number to reason about, not a hypothetical upper bound reserved for some future large deployment. Concrete example from this process: implementing a new multi-source precedence rule as a correlated `EXISTS` subquery re-evaluated per candidate row reads as perfectly reasonable, passes every test at small scale, and is an accidental self-join with no index behind it at scale — the fix (add the missing index, express the aggregation as one `GROUP BY` pass instead of a per-row correlated check) had to be written into the PRD explicitly, or it would have shipped as a footgun that real 10k-device users would have hit, not a theoretical one.
+9. **Check performance against the real schema and real scale, not assumptions.** For every new or changed query: does it use an existing index, or add an unindexed lookup, a new join, or a correlated subquery? Grep `CREATE INDEX` — don't assume a column is indexed just because it looks like a key (check `server/db/db_upgrade.py`/`server/db/schema/app.sql`). Then weigh cost by how often the query runs (once is nothing; every few minutes forever is a standing cost) and by real scale — **production users run 10,000+ devices**, not a homelab handful. A `CurrentScan` with 2-5 rows per device (one per contributing plugin) is routinely 20,000-50,000+ rows in one cycle; reason about that number, not a smaller hopeful one. A correlated `EXISTS`/subquery re-evaluated per outer row is fine *if* the correlated column is indexed — e.g. `current_scan_presence_condition()` (`server/scan/presence.py`) is exactly this shape against `CurrentScan.scanMac`, covered by `idx_currentscan_scanmac`. The real risk is an unindexed correlated lookup: an accidental self-join scanning the full inner table per outer row, which looks fine and passes tests at small scale but isn't at production scale. Check with `EXPLAIN QUERY PLAN` at a realistic row count rather than assuming either way; if it comes back unindexed, a `GROUP BY` aggregate is the usual fix.
 10. **Do a dedicated final-check pass, out loud, before calling it done.** Re-read the whole document end to end and specifically check:
     - Did a correction made mid-document actually propagate everywhere it needed to (the Design subsection *and* Affected Files *and* Tests *and* any execution-plan summary)? A correction landing in one place and not its siblings is worse than never catching it, because now the document silently contradicts itself.
     - Does every "this is the cleanest/simplest real case" claim still hold up if you actually re-read that specific piece of code right now, or was it asserted by pattern-matching a name/category? Re-verify, don't re-assert.

.gemini/skills/scan-pipeline/SKILL.md

@@ -7,57 +7,57 @@ description: Reference for how the scan pipeline actually works — process_scan
 
 ## Scope
 
-This skill covers what happens *after* a plugin's rows land in `CurrentScan` — presence computation, event generation, and session/timeline derivation. It does not cover the plugin-authoring side (manifest, data contract, settings) — see the `plugin-development` skill and `docs/PLUGINS_DEV*.md` for that. It also does not cover the general `*Source` attribution system or SQLite triggers for audit logging — see the `database-patterns` skill (Copilot tree only, as of this writing) for that; the field-authority section below is the scan-pipeline-local half of that bigger system, and the two overlap.
+Covers what happens after a plugin's rows land in `CurrentScan`: presence computation, event generation, session/timeline derivation. Not plugin authoring (manifest, data contract, settings) — see `plugin-development` and `docs/PLUGINS_DEV*.md`. Not the general `*Source` attribution system or SQLite audit triggers — see `database-patterns`; the field-authority section below is the scan-pipeline-local half of that system.
 
-## The core tables/views and their lifetimes
+## Core tables and views
 
-- **`CurrentScan`** — ephemeral scratch table. Populated by `process_plugin_events()` for any plugin whose `config.json` declares `mapped_to_table`, then fully deleted at the end of every `process_scan()` cycle (`DELETE FROM CurrentScan`). Never assume a value written into a `CurrentScan` row is readable outside the cycle it arrived in — by the time a later cycle needs to check "was this row flagged X," the row is gone. This bit a real design (see Gotcha 2 below).
+- **`CurrentScan`** — ephemeral scratch table. `process_plugin_events()` populates it for any plugin whose `config.json` declares `mapped_to_table`; `process_scan()` deletes all rows at the end of every cycle. A value written to a `CurrentScan` row is not readable in a later cycle — the row is gone by then. See Gotcha 2.
 - **`Devices`** — persistent identity + state table.
-- **`Events`** — persistent, append-only log of state-transition events (`New Device`, `Connected`, `Down Reconnected`, `Device Down`, `Disconnected`, `IP Changed`). This *is* the audit trail; `Sessions` is derived from it, not the other way around.
-- **`Sessions`** — not incrementally updated; fully wiped and rebuilt every cycle from a view (see `Convert_Events_to_Sessions` below). Don't reason about it as a live connection state machine — it's a materialized query result recomputed from `Events` each cycle.
-- **`Online_History`** — persistent, one row per scan cycle, feeds the dashboard's online/offline graph. Purely a rollup of `devPresentLastScan`/`devAlertDown`/`devIsSleeping` counts on `DevicesView` — no independent state of its own.
+- **`Events`** — persistent, append-only log of state-transition events (`New Device`, `Connected`, `Down Reconnected`, `Device Down`, `Disconnected`, `IP Changed`). This is the audit trail; `Sessions` is derived from it, not the reverse.
+- **`Sessions`** — fully wiped and rebuilt every cycle from `Convert_Events_to_Sessions` (below), not incrementally updated. Treat it as a materialized query result, not a live connection state machine.
+- **`Online_History`** — one row per scan cycle, feeds the dashboard's online/offline graph. A rollup of `devPresentLastScan`/`devAlertDown`/`devIsSleeping` counts on `DevicesView` — no state of its own.
 
 ## Key views
 
-- **`LatestDeviceScan`** (`server/db/db_upgrade.py`) — `Devices` LEFT JOIN'd to the most recent `CurrentScan` row **per `(scanMac, scanSourcePlugin)` pair**, ranked via `ROW_NUMBER() OVER (PARTITION BY scanMac, scanSourcePlugin ...)`. This is why `update_devices_data_from_scan()` loops over `DISTINCT scanSourcePlugin` and re-queries this view once per plugin: when two plugins report the same device in the same cycle, they are *not* merged into one row before processing — each plugin's contribution is evaluated independently, per field, through the authority mechanism below.
-- **`LatestEventsPerMAC`** — most recent Event per MAC, joined to `Devices` and `CurrentScan`. Used by the "New Connections" query in `insert_events()` to decide whether a device was previously down (→ `Down Reconnected`) or genuinely new (→ `Connected`).
-- **`Convert_Events_to_Sessions`** — the actual definition of "is this device's session still open." **There is no `close_session()`-style function anywhere in this codebase.** A session closes purely as an emergent property: `pair_sessions_events()` sets `evePairEventRowid` on a `New Device`/`Connected`/`Down Reconnected` Event to point at the next `Disconnected`/`Device Down` Event for that MAC, and this view computes `sesStillConnected = 1` exactly when that pairing is still `NULL`. If a session needs to close, the fix is always "make sure the right `Events` row gets inserted" — never a direct `Sessions` mutation (the one exception is `create_new_devices()`'s reconnect-insert, noted below).
-- **`DevicesView`** — adds computed `devIsSleeping`/`devFlapping`/`devStatus` on top of `Devices`. This is what the UI and `insertOnlineHistory()` actually read presence from, not the raw `Devices` table.
+- **`LatestDeviceScan`** (`server/db/db_upgrade.py`) — `Devices` LEFT JOIN'd to the most recent `CurrentScan` row per `(scanMac, scanSourcePlugin)` pair, via `ROW_NUMBER() OVER (PARTITION BY scanMac, scanSourcePlugin ...)`. `update_devices_data_from_scan()` loops over `DISTINCT scanSourcePlugin` and re-queries this view once per plugin: when two plugins report the same device in one cycle, each contribution is evaluated separately, per field, through the authority mechanism below — they are not merged into one row first.
+- **`LatestEventsPerMAC`** — most recent Event per MAC, joined to `Devices` and `CurrentScan`. The "New Connections" query in `insert_events()` uses it to decide whether a device was previously down (→ `Down Reconnected`) or new (→ `Connected`).
+- **`Convert_Events_to_Sessions`** — defines "is this device's session still open." There is no `close_session()` function anywhere in this codebase. A session closes as an emergent property: `pair_sessions_events()` sets `evePairEventRowid` on a `New Device`/`Connected`/`Down Reconnected` Event to point at the next `Disconnected`/`Device Down` Event for that MAC; this view sets `sesStillConnected = 1` exactly when that pairing is `NULL`. To close a session, insert the right `Events` row — never mutate `Sessions` directly (the one exception is `create_new_devices()`'s reconnect-insert, in the call order below).
+- **`DevicesView`** — adds computed `devIsSleeping`/`devFlapping`/`devStatus` on top of `Devices`. The UI and `insertOnlineHistory()` read presence from this, not the raw `Devices` table.
 
-## `process_scan()` call order (`server/scan/session_events.py`) — the order is load-bearing, not incidental
+## `process_scan()` call order (`server/scan/session_events.py`) — order is load-bearing
 
 1. `save_own_device()`, `exclude_ignored_devices()`
-2. `insert_events(db)` — **runs before presence gets updated for this cycle.** Deliberate: the Down/Disconnected/Connected queries need the *previous* cycle's `devPresentLastScan` value to detect a transition (present last cycle but absent now, or vice versa). If this ran after the presence update, every query would see the already-updated value and the edge-triggered design would collapse into either never firing or firing every cycle.
-3. `create_new_devices(db)` — the source comment is explicit: "after create events -> avoid 'connection' event." Brand-new devices get a `New Device` event instead of a `Connected` event, because at step 2 they didn't exist as `Devices` rows for either query to match. Also contains a raw `INSERT INTO Sessions ... sesStillConnected = 1` for devices that already exist but have no currently-open session — the one place outside the `Events`-derived path that writes `Sessions` directly.
+2. `insert_events(db)` — runs before presence updates for this cycle. The Down/Disconnected/Connected queries need the *previous* cycle's `devPresentLastScan` to detect a transition. If this ran after the presence update, every query would see the new value and the edge-triggered design would break — firing never, or every cycle.
+3. `create_new_devices(db)` — runs before presence updates so a brand-new device gets a `New Device` event, not a `Connected` event (it has no `Devices` row yet for step 2's queries to match). Also has a raw `INSERT INTO Sessions ... sesStillConnected = 1` for existing devices with no open session — the one place outside the `Events`-derived path that writes `Sessions` directly.
 4. `update_devices_data_from_scan(db)` — field-level updates for existing devices; see the authority mechanism below.
 5. `update_sync_hub_node`, `update_devLastConnection_from_CurrentScan`
-6. `update_presence_from_CurrentScan(db)` — sets `devPresentLastScan` from bare `CurrentScan` presence *for this cycle* (this becomes the "previous" value step 2 reads on the *next* cycle).
-7. `update_devPresentLastScan_based_on_nics(db)` — NIC/parent-child presence aggregation; can override step 6's result for parent devices.
-8. `update_devPresentLastScan_based_on_force_status(db)` — the user's manual `devForceStatus` override, runs **last**, wins unconditionally over everything above.
+6. `update_presence_from_CurrentScan(db)` — sets `devPresentLastScan` from `CurrentScan` for this cycle (step 2 reads this as "previous" on the *next* cycle).
+7. `update_devPresentLastScan_based_on_nics(db)` — NIC/parent-child presence aggregation; can override step 6 for parent devices.
+8. `update_devPresentLastScan_based_on_force_status(db)` — the user's manual `devForceStatus` override; runs last, wins over everything above.
 9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types`
 10. `pair_sessions_events(db)` — pairs `Events` rows as described above.
-11. `create_sessions_snapshot(db)` — `DELETE FROM Sessions; INSERT INTO Sessions SELECT * FROM Convert_Events_to_Sessions`. This is the point where `Sessions` actually reflects step 10's pairing.
+11. `create_sessions_snapshot(db)` — `DELETE FROM Sessions; INSERT INTO Sessions SELECT * FROM Convert_Events_to_Sessions`. `Sessions` reflects step 10's pairing from here.
 12. `insertOnlineHistory(db)` — dashboard graph rollup.
 13. `skip_repeated_notifications(db)`
-14. `DELETE FROM CurrentScan` — the ephemeral table's entire lifetime is one call to `process_scan()`.
+14. `DELETE FROM CurrentScan` — the table's entire lifetime is one call to `process_scan()`.
 
 ## Field-write authority for scan-derived updates
 
-`update_devices_data_from_scan()` (`server/scan/device_handling.py`) does not blindly overwrite fields from whichever plugin ran most recently. Each trackable field is declared once in `FIELD_SPECS` (`scan_col`, `source_col`, a `priority` list of plugin prefixes, optional `allow_override_if_changed`), and `can_overwrite_field()` uses that plus `get_plugin_authoritative_settings()` (which reads a plugin's own settings for an explicit authority override) to decide, per field per row, whether this plugin's value may replace what's there. The paired `<field>Source` column (`devNameSource`, `devLastIPSource`, etc.) records who currently owns the field. `devMac` itself is never a target of these updates — it's the join key, not a tracked field — so no scan-derived update path can ever alter a device's identity, only its attributes.
+`update_devices_data_from_scan()` (`server/scan/device_handling.py`) does not overwrite fields from whichever plugin ran most recently. Each trackable field is declared once in `FIELD_SPECS` (`scan_col`, `source_col`, a `priority` list of plugin prefixes, optional `allow_override_if_changed`). `can_overwrite_field()` uses that plus `get_plugin_authoritative_settings()` (a plugin's own authority-override setting, if any) to decide, per field per row, whether this plugin's value may replace what's there. The paired `<field>Source` column (`devNameSource`, `devLastIPSource`, etc.) records who currently owns the field. `devMac` is never a target of these updates — it's the join key, not a tracked field — so no scan-derived update can alter a device's identity, only its attributes.
 
-This is the scan-pipeline-local half of a bigger attribution system — see the `database-patterns` skill for `FIELD_SOURCE_MAP` / `server/db/authoritative_handler.py`, the full `*Source` attribution model, and how SQLite triggers consume it for audit logging. Read both if touching anything that writes a `*Source` column.
+This is the scan-pipeline-local half of a bigger attribution system — see `database-patterns` for `FIELD_SOURCE_MAP`/`server/db/authoritative_handler.py`, the full `*Source` model, and the SQLite triggers that consume it for audit logging. Read both before touching anything that writes a `*Source` column.
 
-## Four real gotchas (not hypothetical — all surfaced live during a design review)
+## Gotchas
 
-1. **A "presence" check almost always exists in more than one place.** When adding a per-row signal meaning "don't count this as a live sighting" (e.g. a proposed `scanPresence` column), every query that independently re-derives "is this MAC currently present" from `CurrentScan` has to be updated together — `update_presence_from_CurrentScan()`, the `insert_events()` "New Connections"/"Device Down"/"Disconnected" queries, and the raw `INSERT INTO Sessions` inside `create_new_devices()` all encode that same question separately. Patching one and missing a sibling produces a UI where the device badge, the Events log, and the Sessions timeline each tell a different story for the same device. See `.gemini/internal-docs/PRDs/plugin-import-behavior-controls.md` for the worked example — a `scanPresence = 0` transition that never closed its session because only one of three "is it present" queries had been patched. **Update:** `current_scan_presence_condition()` (`server/scan/presence.py`) now centralizes this for five of those sites — `update_presence_from_CurrentScan()` (both statements), `update_devLastConnection_from_CurrentScan()`, and three of `insert_events()`'s four queries (both `Device Down` variants, `Disconnected`) all call it instead of writing their own `EXISTS (...)`. The remaining two ("New Connections", the raw `Sessions` insert in `create_new_devices()`) still can't use it — they need the actual `scanLastIP`/`scanVendor` *value* off the presence-asserting row via `MIN()`/`GROUP BY`, not just a boolean — so a brand-new presence-adjacent query still has to be checked against both patterns, not assumed to be a bare helper call.
-2. **`CurrentScan` is deleted at the end of every cycle — a per-row flag on it cannot express a decision that needs to survive to a cycle where the row is absent.** Anything that fires specifically *because* a row is missing (`Device Down`, `Disconnected`) cannot read a flag that lived on that now-gone row. If a per-row plugin signal needs to affect behavior beyond the cycle it arrived in, persist it onto the `Devices` row at creation time (e.g. seeding `devAlertDown`/`devAlertEvents` from the row's flag instead of the global `NEWDEV_*` defaults) rather than trying to make the ephemeral table carry it forward.
-3. **`CurrentScan` is not small, and it has an index now — check before assuming otherwise.** Real production users run 10,000+ devices; with the normal one-row-per-contributing-plugin pattern (see `LatestDeviceScan` above), a single cycle's `CurrentScan` is routinely 20,000-50,000+ rows, not the few hundred a homelab install might suggest. `idx_currentscan_scanmac` was added to `server/db/db_upgrade.py:ensure_CurrentScan()` (and mirrored in `server/db/schema/app.sql`) specifically because every `scanMac`-keyed lookup in this file was a full table scan without it — confirmed via `EXPLAIN QUERY PLAN` before the fix. Note `ensure_CurrentScan()` itself (the `DROP TABLE`/`CREATE TABLE`) only runs once, at app startup (`DB.initDB()`, called once from `server/__main__.py`) — don't confuse this with the per-cycle `DELETE FROM CurrentScan` in point 1 above, which clears rows but doesn't touch the table or its index. The index is built once and maintained incrementally, not rebuilt every cycle.
-4. **`server/plugins/sync/sync.py` bypasses this entire pipeline on purpose, twice — a permanent exception, not a bug.** It fires its own direct `INSERT OR IGNORE INTO Events (... 'New Device' ...)` for newly-seen synced devices (hardcoded `evePendingAlertEmail = 1`, no `scanNotificationMode`/quiet awareness), and in `carbon-copy` mode its own raw `Devices` UPSERT via `ON CONFLICT(devMac) DO UPDATE` — both deliberately skipping `create_new_devices()`/`update_devices_data_from_scan()`/`can_overwrite_field()` (`sync.py`'s own comments document this as intentional: "Node is fully authoritative in this mode"). It *is* a normal `mapped_to_table: CurrentScan` plugin for its presence contribution, so `IMPORT_ON`/`scanPresence` apply to it exactly like any other plugin — but its two direct-write paths would silently ignore `scanNotificationMode = 'quiet'` or `scanCreatesDevice = 0` if `sync` ever adopted either. Keep this in mind whenever touching the generic pipeline and assuming every `Events`/`Devices` write went through it — `sync.py` is the one place that doesn't.
-
-**Correction: `app.sql` is not dead code** — an earlier version of this note called it "otherwise-unused." Checked further: `install/production-filesystem/entrypoint.d/25-first-run-db.sh` pipes it straight into `sqlite3` to bootstrap a brand-new database on first install, and `scripts/db_cleanup/regenerate-database.sh` uses it too. `CurrentScan`, `Parameters`, and `Settings` are safe from drift because each has a dedicated `ensure_X()` function (`server/db/db_upgrade.py`) that unconditionally drops and recreates the table on every startup, superseding whatever `app.sql` bootstrapped. `Plugins_Language_Strings` gets the same unconditional drop/recreate, but inside the shared `ensure_plugins_tables()`, not a dedicated function of its own. `AppEvents` gets an equivalent drop/recreate too, via a different mechanism — `AppEvent_obj.__init__()` (`server/workflows/app_events.py`) drops and recreates it on every startup, independent of `db_upgrade.py`. `Devices` has no drop/recreate, but `server/database.py` has 18 explicit `ensure_column()` calls that backfill any column missing from an older `app.sql` snapshot on every startup. **`Events`, `Sessions`, and `Notifications`** — the three tables that genuinely had neither safety net — **now have the same backfill treatment**, per `scan-pipeline-hardening.md` Design §3 (implemented): `ensure_table_columns()` (`server/db/db_upgrade.py`), driven by one Python column-list constant per table (`server/db/schema_columns.py`) that's also diffed against `app.sql` in CI (`test/db/test_schema_drift_guard.py`), so drift between the two is caught rather than silently shipping. `AppEvents`/`Notifications` each also have a *second* schema-definition surface beyond `app.sql` worth knowing about — their own inline `CREATE TABLE IF NOT EXISTS` in `server/workflows/app_events.py`/`server/models/notification_instance.py` respectively — kept in sync via the same drift-check test. Any *new* query added here should still be checked with `EXPLAIN QUERY PLAN` at a realistic row count rather than assumed fine because it "looks like the existing queries" — several of those existing queries were themselves unindexed scans until this was caught. A correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable and be quadratic at this scale.
+1. **A "presence" check exists in more than one place.** A per-row signal meaning "don't count this as a live sighting" (e.g. `scanPresence`) has to reach every query that independently re-derives "is this MAC currently present" from `CurrentScan`. `current_scan_presence_condition()` (`server/scan/presence.py`) centralizes that check for five sites: `update_presence_from_CurrentScan()` (both statements), `update_devLastConnection_from_CurrentScan()`, and three of `insert_events()`'s four queries (both `Device Down` variants, `Disconnected`). Two sites can't use it: the "New Connections" query and the raw `Sessions` insert in `create_new_devices()` need the actual `scanLastIP`/`scanVendor` value off the presence-asserting row via `MIN()`/`GROUP BY`, not just a boolean. Check any new presence-adjacent query against both patterns — a bare helper call isn't always enough.
+2. **`CurrentScan` is deleted at the end of every cycle — a per-row flag on it can't express a decision that needs to survive to a cycle where the row is gone.** Anything that fires because a row is *missing* (`Device Down`, `Disconnected`) can't read a flag that lived on that row. A per-row plugin signal that needs to affect behavior beyond its own cycle has to persist onto the `Devices` row at creation time (e.g. seeding `devAlertDown`/`devAlertEvents` from the row's flag instead of the global `NEWDEV_*` defaults), not ride on the ephemeral table.
+3. **`CurrentScan` is not small, and it's indexed on `scanMac`.** Real production users run 10,000+ devices; with one row per contributing plugin (see `LatestDeviceScan` above), a single cycle's `CurrentScan` is routinely 20,000-50,000+ rows. `idx_currentscan_scanmac` (`server/db/db_upgrade.py:ensure_CurrentScan()`, mirrored in `server/db/schema/app.sql`) covers every `scanMac`-keyed lookup in this file. `ensure_CurrentScan()`'s `DROP TABLE`/`CREATE TABLE` runs once, at app startup (`DB.initDB()`, `server/__main__.py`) — don't confuse this with the per-cycle `DELETE FROM CurrentScan` in point 1, which clears rows but leaves the table and its index in place.
+4. **`server/plugins/sync/sync.py` bypasses this pipeline on purpose, twice — a permanent exception, not a bug.** It fires its own direct `INSERT OR IGNORE INTO Events (... 'New Device' ...)` for newly-seen synced devices (hardcoded `evePendingAlertEmail = 1`, no `scanNotificationMode` awareness), and in `carbon-copy` mode its own raw `Devices` UPSERT via `ON CONFLICT(devMac) DO UPDATE` — both skip `create_new_devices()`/`update_devices_data_from_scan()`/`can_overwrite_field()` (`sync.py`'s own comments: "Node is fully authoritative in this mode"). It's a normal `mapped_to_table: CurrentScan` plugin for its presence contribution, so `IMPORT_ON`/`scanPresence` apply to it like any other plugin — but its two direct-write paths ignore `scanNotificationMode = 'quiet'` or `scanCreatesDevice = 0`. Don't assume every `Events`/`Devices` write goes through the generic pipeline — `sync.py` doesn't.
+5. **A blank/null-equivalent `scanMac` can create a phantom `Devices` row.** `create_new_devices()`'s two creation-path queries filter `scanMac NOT IN (NULL_EQUIVALENTS_SQL)` (`server/scan/device_handling.py`, `const.NULL_EQUIVALENTS_SQL`) as a backstop, because `scanCreatesDevice` defaults to `1` — any plugin reporting a row with no real MAC, without setting `scanCreatesDevice = 0` itself, would otherwise create a `devMac = ''` device, and every other blank-MAC row from every other plugin would then silently write onto it. The filter doesn't replace `scanCreatesDevice = 0` as the correct thing for a plugin to set on such rows; it keeps a MAC-less row inert when some other plugin forgets to. Check any new creation-adjacent query against blank `scanMac` too.
+6. **`app.sql` is not dead code.** `install/production-filesystem/entrypoint.d/25-first-run-db.sh` pipes it into `sqlite3` to bootstrap a brand-new database on first install; `scripts/db_cleanup/regenerate-database.sh` uses it too. `CurrentScan`, `Parameters`, and `Settings` are safe from drift: each has a dedicated `ensure_X()` function (`server/db/db_upgrade.py`) that drops and recreates the table on every startup, superseding whatever `app.sql` bootstrapped. `Plugins_Language_Strings` gets the same treatment inside the shared `ensure_plugins_tables()`. `AppEvents` gets its own drop/recreate via `AppEvent_obj.__init__()` (`server/workflows/app_events.py`), independent of `db_upgrade.py`. `Devices` has no drop/recreate, but `server/database.py` has 18 explicit `ensure_column()` calls that backfill any column missing from an older `app.sql` snapshot on every startup. `Events`, `Sessions`, and `Notifications` get the same backfill via `ensure_table_columns()` (`server/db/db_upgrade.py`), driven by one Python column-list constant per table (`server/db/schema_columns.py`) that's diffed against `app.sql` in CI (`test/db/test_schema_drift_guard.py`). `AppEvents`/`Notifications` each also have a second schema-definition surface — their own inline `CREATE TABLE IF NOT EXISTS` in `server/workflows/app_events.py`/`server/models/notification_instance.py` — kept in sync by the same drift-check test. Check any new query here with `EXPLAIN QUERY PLAN` at a realistic row count rather than assuming it's fine because it resembles an existing one — a correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable while actually being quadratic at this scale.
 
 ## When to read this vs. other docs/skills
 
-- Writing or reviewing a plugin's `config.json`/data contract → `plugin-development` skill, `docs/PLUGINS_DEV*.md`. This skill is about what happens *after* a plugin's rows land in `CurrentScan`, not the plugin-authoring contract itself.
-- Devices-table write paths, `*Source` attribution, audit/history logging, SQLite triggers → `database-patterns` skill.
-- Actually implementing a change here → read the relevant function in `server/scan/session_events.py` / `server/scan/device_handling.py` directly before trusting this skill's line-number references; they're a map, not a guarantee, and will drift as the code moves.
+- Writing or reviewing a plugin's `config.json`/data contract → `plugin-development`, `docs/PLUGINS_DEV*.md`. This skill covers what happens *after* a plugin's rows land in `CurrentScan`, not the authoring contract.
+- Devices-table write paths, `*Source` attribution, audit/history logging, SQLite triggers → `database-patterns`.
+- Implementing a change here → read the actual function in `server/scan/session_events.py`/`server/scan/device_handling.py` first; this skill's line numbers are a map, not a guarantee, and drift as the code moves.

.gemini/skills/skill-hygiene/SKILL.md

@@ -0,0 +1,44 @@
+---
+name: skill-hygiene
+description: Read before writing or editing any SKILL.md in this repo (.claude/.gemini/.github skills trees). Covers the two standing rules for skill prose - state current behavior only, and prefer plain, short wording - plus the grep sweep to run before calling a skill clean.
+---
+
+# Skill Hygiene
+
+## What a skill is, and isn't
+
+A skill is a live reference for how the system works *now*. It is not a lab notebook, a changelog, or a PRD. History, corrections, and discovery narratives belong in PRDs (`.gemini/internal-docs/PRDs/`, see `prd-writing`) or commit messages — never in a skill body. If a skill needs to change because the code changed, edit it in place; don't leave a trace of what it used to say.
+
+This is a different rule from `prd-writing`'s "leave a visible trail of corrections" — that rule is specifically for PRDs, where the trail is the point. A skill has no reader who benefits from seeing your editing history; it only has readers who need the current fact, stated plainly.
+
+## Rule 1: state current behavior only
+
+Cut anything that narrates the past instead of stating the present:
+
+| Smell | Why it's a problem | Fix |
+|---|---|---|
+| `"Correction: X — an earlier version of this note said Y"` | Narrates an edit, not a fact | Just state the current fact about X |
+| `"(as of 2026-07-04)"` | Hedges instead of committing to the fact | State it as true now; update the line when it stops being true |
+| `"This shipped for real in devParentRelType"` / `"...caught in review"` / `"...caught mid-review"` | Tells the story of finding a bug instead of the rule that prevents it | State the rule; use the real example as a plain parenthetical if it helps, without the origin story |
+| `"Two real examples from this exact process:"` / `"during a past audit"` | Frames the skill as a diary of one session | Turn the anecdote into a timeless illustration, or drop it |
+| `"X now does Y"` / `"X previously did Y"` / `"used to be"` / `"no longer"` when describing *the skill's own past text* | Describes the skill's edit history, not the system | Delete — say what's true now, full stop |
+
+`"no longer"` describing real *system* behavior (e.g. "there is no longer a retry loop here") is fine — that's a fact about the code, not about the skill. The test is: does this sentence describe the codebase, or does it describe a previous version of this document?
+
+## Rule 2: plain words, fewer words
+
+If a shorter or simpler phrasing says the same thing, use it. Cut qualifiers that don't change the meaning ("actually," "really," "genuinely," "in this exact process"). Prefer a plain verb over a nominalization. A dense skill with real information beats a padded one — trim narration and hedging before trimming facts.
+
+## Sweep before calling a skill clean
+
+Run this across `.claude/skills/`, `.gemini/skills/`, `.github/skills/` (or a single file being edited):
+
+```bash
+grep -rniE "as of 202|caught in review|caught mid-review|correction:|correction \(|shipped for real|previously|used to be|no longer|originally|was later|historically|in the past|distilled from|real mistake|it turned out|turns out|discovered that|during a past" .claude/skills/ .gemini/skills/ .github/skills/
+```
+
+Read every hit in context — some are legitimate (a rule instructing PRD authors to write correction trails, or "previously down" describing device state, are not violations). Fix the ones that narrate the skill's own history instead of the system's current behavior.
+
+## Keep the three trees in sync
+
+Most skills exist as three near-identical copies (`.claude/skills/<name>/SKILL.md`, `.gemini/skills/<name>/SKILL.md`, `.github/skills/<name>/SKILL.md` — see `.gemini/skills/skills-index/SKILL.md` for the pairing map). When a hygiene fix changes a skill's body, apply the same fix to all paired copies so they stay identical (frontmatter `name`/`description` may differ per tree's own convention; the body should not). `scripts/check_skill_pairs.py`'s `GROUPS` list only flags when some-but-not-all paired files changed in a diff — it doesn't check the bodies actually match, so a manual diff after editing is still worth it.

.gemini/skills/skills-index/SKILL.md

@@ -9,7 +9,7 @@ Three AI assistants are configured for this project, each with their own skill d
 
 - **Gemini CLI** → `.gemini/skills/`
 - **GitHub Copilot** → `.github/skills/`
-- **Claude Code** → `.claude/skills/` (currently mirrors only the 3 highest-value skills below, not the full set)
+- **Claude Code** → `.claude/skills/` (mirrors most, not all, of the shared skills below — see the table's Claude Skill column for which)
 
 Skills with the same purpose exist in more than one, sometimes under different names and with different depth. This index maps them so you can find the richer version when needed. A CI check (`scripts/check_skill_pairs.py`, run as `check-skill-pairs` in `.github/workflows/code-checks.yml`) flags PRs that touch some but not all files in a mirrored group - non-blocking, since some divergence is intentional.
 
@@ -30,7 +30,9 @@ Skills with the same purpose exist in more than one, sometimes under different n
 | Logging | `logging-standards` | `logging-standards` | — | `mylog` levels, message format, what not to log |
 | Scan pipeline internals | `scan-pipeline` | `scan-pipeline` | `scan-pipeline` | `process_scan()` call order and why it's load-bearing, `CurrentScan`/`Events`/`Sessions`/`DevicesView` relationships, how a session actually closes (no `close_session()` exists), and the `FIELD_SPECS` field-write authority mechanism. Complements `database-patterns` (Devices write-path/`*Source` attribution) rather than duplicating it. |
 | Database patterns | `database-patterns` | `database-patterns` | `database-patterns` | Devices table write-path inventory, the `FIELD_SOURCE_MAP`/`*Source` attribution system in `server/db/authoritative_handler.py`, SQLite trigger vs. Python-hook tradeoffs, and event-sourced vs. snapshot audit logging. |
-| PRD writing | `prd-writing` | `prd-writing` | `prd-writing` | Methodology for writing a design doc: challenge the idea, verify every claim against actual code, trace every downstream consumer of a new mechanism, evaluate performance impact against the real schema/indexes, record rejected alternatives and open-issue decisions explicitly, final-check pass before done. Distilled from the `plugin-import-behavior-controls` PRD process, including real mistakes caught mid-review. |
+| PRD writing | `prd-writing` | `prd-writing` | `prd-writing` | Methodology for writing a design doc: challenge the idea, verify every claim against actual code, trace every downstream consumer of a new mechanism, evaluate performance impact against the real schema/indexes, record rejected alternatives and open-issue decisions explicitly, final-check pass before done. |
+| UX/frontend design | `ux-design-patterns` | `ux-design-patterns` | `ux-design-patterns` | Don't invent new UX behavior/visual patterns unless a PRD calls for it - search `front/` for an existing pattern first and reuse it. Priority order for design tradeoffs when several options are reasonable: existing behavior > intuitiveness > information density > usability > utility > uniqueness > industry practices > generic UI. |
+| Skill hygiene | `skill-hygiene` | `skill-hygiene` | `skill-hygiene` | Read before writing/editing any SKILL.md. Two standing rules: state current behavior only (no "Correction:", no "as of <date>", no "caught in review" narration - that trail belongs in PRDs), and prefer plain, short wording. Includes the grep sweep to run before calling a skill clean. |
 
 ---
 

.gemini/skills/ux-design-patterns/SKILL.md

@@ -0,0 +1,35 @@
+---
+name: ux-design-patterns
+description: Read before adding or changing any front/ UI element - a control, layout, button, or interaction pattern. Covers the don't-invent-new-UX-without-a-PRD rule and the priority order for design tradeoffs (existing behavior > intuitiveness > information density > usability > utility > uniqueness > industry practices > generic UI).
+---
+
+# UX / Frontend Design Patterns
+
+## Core principle: reuse before inventing
+
+Don't introduce new UX behavior or visual patterns unless a PRD explicitly calls for it. Before building any new UI element, search the existing frontend for a pattern that already solves this exact need, and reuse its markup/CSS/behavior instead of inventing a new one.
+
+Real, recent example: a presence-page Prev/Next pager was first built with custom `<button class="btn btn-xs">` elements floated in a `.box-header`. The DataTables pagination pattern (`dataTables_wrapper` / `dataTables_paginate` / `ul.pagination` / `li.paginate_button.previous|next`) already existed elsewhere in the app and does the exact same job. The custom version looked visually broken in the actual UI and had to be reimplemented using the existing pattern once that was caught in manual testing - reusing it also picked up dark-mode theming (`front/css/dark-patch.css`'s `.pagination li > a` / `.disabled` rules) for free, which the hand-rolled version didn't have. Grep first: e.g. `grep -rn "pagination\|paginate_button" front/` before adding a "previous/next" control of your own; the same applies to modals, filter inputs, badges, tooltips, tables - anything that already has an established shape somewhere in `front/`.
+
+## Priority order for design decisions
+
+When several options are all locally reasonable, resolve the choice in this order - highest wins on conflict:
+
+1. **Existing behavior** - what does this codebase already do for the same or a similar need? Copy it.
+2. **Intuitiveness** - will a user already familiar with the rest of the app understand this without being told?
+3. **Information density** - does it show what's needed without wasting space or hiding what matters?
+4. **Usability** - is it easy and low-friction to actually use (reachability, click count, error tolerance)?
+5. **Utility** - does it solve the real problem, not just resemble a solution?
+6. **Uniqueness** - is this the app's own distinct answer, used only where nothing generic fits well?
+7. **Industry practices** - conventions users bring in from other apps.
+8. **Generic UI** - a default/framework-provided look, used only when nothing above applies.
+
+This list exists to end debates quickly, not to be argued from the bottom up. The reason it's written down is that #1 is exactly the step that gets skipped under time pressure - checking it first is meant to be fast, not a detour.
+
+## Practical checklist before building a new UI element
+
+1. Grep `front/js/`, `front/css/`, `front/php/` for an existing implementation of the same interaction - a table, a pager, a filter box, a modal, a badge, a status indicator.
+2. If found, reuse its markup and CSS classes directly rather than writing new ones - matching classes inherit theming (dark mode, responsive breakpoints) a new hand-rolled version won't have.
+3. If nothing fits, check whether the PRD driving this change actually calls for new UX. If it doesn't, that's a signal to look harder for an existing pattern, not license to invent one.
+4. If a new pattern is genuinely warranted and the PRD says so, design it using the priority order above, and record the choice and why existing patterns didn't fit in the PRD - the next change will hit the same fork and shouldn't have to re-derive the answer.
+5. Verify visually in a real browser/devcontainer before calling it done. A change that "should work" per the markup isn't confirmed until it's actually rendered - matches the project's general "test the golden path in a browser" rule for frontend changes.

.github/skills/code-standards/SKILL.md

@@ -25,6 +25,8 @@ description: NetAlertX coding standards and conventions. Use this when writing c
 - all code needs to be scalable to handle large networks with thousands of devices (10k+) without performance degradation
 - no inline imports, all imports must be at the top of the file
 - when using `server/logger.py` `mylog()`, only use valid levels: `none`, `minimal`, `verbose`, `debug`, `trace`; invalid levels silently degrade to `none`
+- every Python function/method needs a succinct docstring describing its current use and behavior — not what changed or why (see Docstrings section below)
+- before adding a new frontend language string, search `front/php/templates/language/en_us.json` for an existing key with the same text/purpose and reuse it — don't add a near-duplicate key just because it's needed on a new page (see Language Strings section below)
 
 
 ## File Length
@@ -80,6 +82,33 @@ Use timeNowUTC(as_string=False) for datetime operations (scheduling, comparisons
 
 Use sanitizers from `server/helper.py` before storing user input. MAC addresses are always lowercased and normalized. IP addresses should be validated.
 
+## Docstrings
+
+Every Python function/method gets a docstring — one or two sentences, describing what it does and how it's used *right now*. Not a changelog:
+
+```python
+# Correct
+def count_children_by_parent_mac(devices):
+    """Return {parentMac: childCount} for the given device list, keyed by devParentMAC."""
+
+# Wrong — narrates the diff instead of the current behavior
+def count_children_by_parent_mac(devices):
+    """Replaces the old get_number_of_children() to fix the O(n^2) scan."""
+```
+
+That history belongs in the commit message or PR description, not the docstring — it rots the moment the next change lands. Keep it succinct; only go past a couple of lines when the contract genuinely needs it (non-obvious return shape, units, a caller-visible side effect).
+
+## Language Strings — Reuse Before Adding (DRY)
+
+Before adding a new key to `front/php/templates/language/en_us.json`, grep it for an existing key with the same text or purpose and reuse that key instead of adding a near-duplicate:
+
+```bash
+grep -n "\"Gen_" front/php/templates/language/en_us.json   # generic, reusable strings
+grep -n "Next\|Previous\|Showing" front/php/templates/language/en_us.json
+```
+
+Prefer the generic `Gen_*` keys (e.g. `Gen_Prev`, `Gen_Next`) over a page-scoped name (`Presence_Page_Prev`) for genuinely generic UI text — a future page needing the same label should find it already there. Only add a new key when nothing existing fits; only that one file needs the addition — `getString()`/`lang()` fall back to the English string for any locale missing a key, so the other ~23 locale files don't need touching.
+
 ## Devcontainer Constraints
 
 - Never `chmod` or `chown` during operations

.github/skills/database-patterns/SKILL.md

@@ -9,7 +9,7 @@ description: NetAlertX database architecture patterns. Use this when designing f
 
 Before implementing any feature that reads or writes the `Devices` table, audit ALL write paths. The table is modified from many locations — missing one path is a correctness bug.
 
-**Known production write paths (as of 2026-07-04):**
+**Known production write paths:**
 
 | File | Function | Fields written |
 |---|---|---|

.github/skills/plugin-readme/SKILL.md

@@ -21,7 +21,7 @@ Exception: call out a *specific* setting by name, in prose, only when its behavi
 
 ## Verify against the actual code first
 
-Read `config.json` (`unique_prefix`, `plugin_type`, `data_source`, `settings`) and the plugin's script before writing anything - don't guess at mechanism from the plugin's name alone. Real bugs found this way during a past audit: `dig_scan/README.md` described the `nbtscan` utility (copy-paste from a sibling plugin); `adguard_import/README.md` was a byte-for-byte copy of `__template/README.md`, never actually written.
+Read `config.json` (`unique_prefix`, `plugin_type`, `data_source`, `settings`) and the plugin's script before writing anything - don't guess at mechanism from the plugin's name alone. A README copied from a sibling plugin or left as the unedited `__template/README.md` describes the wrong plugin's behavior.
 
 ## Backfilling missing "Other info"
 

.github/skills/plugin-run-development/SKILL.md

@@ -44,7 +44,8 @@ server/plugins/<code_name>/
 - `<PREF>_CMD`: script path
 - `<PREF>_RUN_TIMEOUT`: timeout in seconds — **this is enforced by the core plugin runner as the whole script's kill-timeout** (`server/plugin.py` passes it straight to `subprocess(..., timeout=...)`). It is not a safe per-HTTP-call timeout — don't reuse it for individual network calls in a loop, or one slow call can burn the whole budget and get the process killed before it writes its result file.
 - `<PREF>_WATCH`: columns to watch for changes
-- `<PREF>_IMPORT_ON`: optional — gates whether this run's rows get promoted into `CurrentScan` (only relevant if `mapped_to_table: "CurrentScan"`). See `docs/PLUGINS_DEV_DATA_CONTRACT.md` for the related per-row `scanCreatesDevice`/`scanNotificationMode`/`scanPresence` columns.
+- `<PREF>_IMPORT_ON`: optional — gates whether this run's rows get promoted into `CurrentScan` (only relevant if `mapped_to_table: "CurrentScan"`). See `docs/PLUGINS_IMPORT_BEHAVIOR.md` for the related per-row `scanCreatesDevice`/`scanNotificationMode`/`scanPresence` columns.
+- **`dataType` and `default_value` must agree.** `dataType: "array"`/`"object"` needs a real JSON literal for `default_value` (`'["default"]'`), not a bare string (`"default"`). `setting_value_to_python_type()` (`server/helper.py`) `json.loads()`s the default at runtime; a bare string fails silently — logged, and `[]` is returned instead of your default (e.g. `devParentRelType`, `UI_theme`, `UI_TOPOLOGY_ORDER`). If `elementOptions` already sets `multiple`/`orderable: "false"`, the setting is scalar — use `dataType: "string"` instead.
 
 ## Data Contract
 

.github/skills/prd-writing/SKILL.md

@@ -11,12 +11,12 @@ Triggered by: "write a PRD", "draft a design doc", "spec out this feature", "cre
 
 ## Core principle: a PRD is a claim-verification exercise, not a writing exercise
 
-Every sentence that asserts something about how the code currently works must be checked against the actual code before it goes in — not written from memory, not inferred from a plugin's name or reputation, not assumed because it sounds plausible. Two real, caught-in-review examples from this exact process:
+Every sentence that asserts something about how the code currently works must be checked against the actual code before it goes in — not written from memory, not inferred from a plugin's name or reputation, not assumed because it sounds plausible. Two failure patterns to watch for:
 
-- A draft claimed "plugin X's rows are the cleanest case for a static presence-flag value" — reasonable-sounding, and wrong. Reading the actual script showed it already reports a live per-row state field and computes an equivalent boolean internally that was simply never wired up. The claim was never checked against the script, only against the plugin's category ("reservation-style").
-- A draft claimed "no changes needed here" for two queries when adding a new presence column, on the reasoning that they were already "inert" for the new case. True for one sub-case (a device that starts absent), false for the transition sub-case (a device going from online to newly-suppressed) — because those two queries used a different existence check than the one already patched. It was missed for a full turn, until a direct question ("does this handle the transition where a device *was* online?") forced a re-trace of the actual call graph.
+- Calling a plugin "the cleanest case" for some behavior based on its category alone. Read the actual script — it may already report a live per-row signal and compute an equivalent value internally that was never wired up.
+- Claiming "no changes needed" for a query based on one sub-case (e.g. a device that starts absent) without checking the transition sub-case (a device going from online to newly-suppressed) — the two sub-cases can use different existence checks, so a fix covering one can silently miss the other.
 
-Both mistakes were plausible, well-written, and wrong. Neither would have survived actually reading the code first.
+Both are plausible, well-written, and wrong. Reading the code first catches both.
 
 ## Process
 
@@ -28,7 +28,7 @@ Both mistakes were plausible, well-written, and wrong. Neither would have surviv
 6. **Force every open question to an explicit decision**, even if the decision is "accept as-is for v1, revisit if feedback says otherwise." An open question left unresolved in a PRD gets silently decided by whoever implements it — usually differently than anyone actually intended.
 7. **Write the test plan as part of the PRD, not after.** Concrete test cases — naming real functions/queries, not "add tests for X" — force you to notice design gaps you'd otherwise miss; the moment you try to write "assert Y happens" and realize the current design can't produce Y is often the first time the gap becomes visible. Check the repo for an existing test pattern for this shape of change before inventing a new one (e.g. a prior presence-logic bug fixed via `test/db_test_helpers.py` fixtures is the template for the next one, not a reason to build new test infrastructure).
 8. **Ask explicitly whether validating this needs real end-to-end infrastructure** (a new or modified plugin, a UI click-through) or whether synthetic unit-level fixtures suffice — don't assume either way. Check whether the functions under test take a DB connection/dict/list as a parameter (testable in isolation, no real plugin needed) or require a real file on disk (harder to fake, may need one).
-9. **Evaluate performance impact against the schema that actually exists, not the schema you'd expect, and against real deployment scale, not an imagined one.** For every new or modified query: does it reuse an existing index, or does it add an unindexed lookup, a new join, or a correlated subquery? Check for real — don't assume a column is indexed just because it looks identity-like (`CurrentScan.scanMac` looked like exactly the kind of column that should have an index; grepping for `CREATE INDEX` showed this codebase never gave it one, and neither did the first draft of the design that needed it — since fixed, `idx_currentscan_scanmac` now exists in `server/db/db_upgrade.py:ensure_CurrentScan()`, so check whether a later PRD's problem is already mitigated before assuming it's new). Then multiply the per-query cost by two things: how often it runs (a full scan inside a loop that fires once is nothing; the same scan inside a cycle that reruns every few minutes forever is a standing cost, permanently), and the actual scale this project runs at — **known real production users run 10,000+ devices** (confirmed directly by the project owner, not a guess or an inference from `CLAUDE.md`'s "homelabs, MSPs, and NOCs" framing). At that scale, a `CurrentScan` populated at 2-5 rows per device (one per contributing plugin, the normal case) is routinely 20,000-50,000+ rows in a single cycle — treat that as the number to reason about, not a hypothetical upper bound reserved for some future large deployment. Concrete example from this process: implementing a new multi-source precedence rule as a correlated `EXISTS` subquery re-evaluated per candidate row reads as perfectly reasonable, passes every test at small scale, and is an accidental self-join with no index behind it at scale — the fix (add the missing index, express the aggregation as one `GROUP BY` pass instead of a per-row correlated check) had to be written into the PRD explicitly, or it would have shipped as a footgun that real 10k-device users would have hit, not a theoretical one.
+9. **Check performance against the real schema and real scale, not assumptions.** For every new or changed query: does it use an existing index, or add an unindexed lookup, a new join, or a correlated subquery? Grep `CREATE INDEX` — don't assume a column is indexed just because it looks like a key (check `server/db/db_upgrade.py`/`server/db/schema/app.sql`). Then weigh cost by how often the query runs (once is nothing; every few minutes forever is a standing cost) and by real scale — **production users run 10,000+ devices**, not a homelab handful. A `CurrentScan` with 2-5 rows per device (one per contributing plugin) is routinely 20,000-50,000+ rows in one cycle; reason about that number, not a smaller hopeful one. A correlated `EXISTS`/subquery re-evaluated per outer row is fine *if* the correlated column is indexed — e.g. `current_scan_presence_condition()` (`server/scan/presence.py`) is exactly this shape against `CurrentScan.scanMac`, covered by `idx_currentscan_scanmac`. The real risk is an unindexed correlated lookup: an accidental self-join scanning the full inner table per outer row, which looks fine and passes tests at small scale but isn't at production scale. Check with `EXPLAIN QUERY PLAN` at a realistic row count rather than assuming either way; if it comes back unindexed, a `GROUP BY` aggregate is the usual fix.
 10. **Do a dedicated final-check pass, out loud, before calling it done.** Re-read the whole document end to end and specifically check:
     - Did a correction made mid-document actually propagate everywhere it needed to (the Design subsection *and* Affected Files *and* Tests *and* any execution-plan summary)? A correction landing in one place and not its siblings is worse than never catching it, because now the document silently contradicts itself.
     - Does every "this is the cleanest/simplest real case" claim still hold up if you actually re-read that specific piece of code right now, or was it asserted by pattern-matching a name/category? Re-verify, don't re-assert.

.github/skills/scan-pipeline/SKILL.md

@@ -7,57 +7,57 @@ description: NetAlertX scan pipeline internals — process_scan() call order and
 
 ## Scope
 
-This skill covers what happens *after* a plugin's rows land in `CurrentScan` — presence computation, event generation, and session/timeline derivation. It does not cover the plugin-authoring side (manifest, data contract, settings) — see the `plugin-development` skill and `docs/PLUGINS_DEV*.md` for that. It also does not cover the general `*Source` attribution system or SQLite triggers for audit logging — see the `database-patterns` skill (Copilot tree only, as of this writing) for that; the field-authority section below is the scan-pipeline-local half of that bigger system, and the two overlap.
+Covers what happens after a plugin's rows land in `CurrentScan`: presence computation, event generation, session/timeline derivation. Not plugin authoring (manifest, data contract, settings) — see `plugin-development` and `docs/PLUGINS_DEV*.md`. Not the general `*Source` attribution system or SQLite audit triggers — see `database-patterns`; the field-authority section below is the scan-pipeline-local half of that system.
 
-## The core tables/views and their lifetimes
+## Core tables and views
 
-- **`CurrentScan`** — ephemeral scratch table. Populated by `process_plugin_events()` for any plugin whose `config.json` declares `mapped_to_table`, then fully deleted at the end of every `process_scan()` cycle (`DELETE FROM CurrentScan`). Never assume a value written into a `CurrentScan` row is readable outside the cycle it arrived in — by the time a later cycle needs to check "was this row flagged X," the row is gone. This bit a real design (see Gotcha 2 below).
+- **`CurrentScan`** — ephemeral scratch table. `process_plugin_events()` populates it for any plugin whose `config.json` declares `mapped_to_table`; `process_scan()` deletes all rows at the end of every cycle. A value written to a `CurrentScan` row is not readable in a later cycle — the row is gone by then. See Gotcha 2.
 - **`Devices`** — persistent identity + state table.
-- **`Events`** — persistent, append-only log of state-transition events (`New Device`, `Connected`, `Down Reconnected`, `Device Down`, `Disconnected`, `IP Changed`). This *is* the audit trail; `Sessions` is derived from it, not the other way around.
-- **`Sessions`** — not incrementally updated; fully wiped and rebuilt every cycle from a view (see `Convert_Events_to_Sessions` below). Don't reason about it as a live connection state machine — it's a materialized query result recomputed from `Events` each cycle.
-- **`Online_History`** — persistent, one row per scan cycle, feeds the dashboard's online/offline graph. Purely a rollup of `devPresentLastScan`/`devAlertDown`/`devIsSleeping` counts on `DevicesView` — no independent state of its own.
+- **`Events`** — persistent, append-only log of state-transition events (`New Device`, `Connected`, `Down Reconnected`, `Device Down`, `Disconnected`, `IP Changed`). This is the audit trail; `Sessions` is derived from it, not the reverse.
+- **`Sessions`** — fully wiped and rebuilt every cycle from `Convert_Events_to_Sessions` (below), not incrementally updated. Treat it as a materialized query result, not a live connection state machine.
+- **`Online_History`** — one row per scan cycle, feeds the dashboard's online/offline graph. A rollup of `devPresentLastScan`/`devAlertDown`/`devIsSleeping` counts on `DevicesView` — no state of its own.
 
 ## Key views
 
-- **`LatestDeviceScan`** (`server/db/db_upgrade.py`) — `Devices` LEFT JOIN'd to the most recent `CurrentScan` row **per `(scanMac, scanSourcePlugin)` pair**, ranked via `ROW_NUMBER() OVER (PARTITION BY scanMac, scanSourcePlugin ...)`. This is why `update_devices_data_from_scan()` loops over `DISTINCT scanSourcePlugin` and re-queries this view once per plugin: when two plugins report the same device in the same cycle, they are *not* merged into one row before processing — each plugin's contribution is evaluated independently, per field, through the authority mechanism below.
-- **`LatestEventsPerMAC`** — most recent Event per MAC, joined to `Devices` and `CurrentScan`. Used by the "New Connections" query in `insert_events()` to decide whether a device was previously down (→ `Down Reconnected`) or genuinely new (→ `Connected`).
-- **`Convert_Events_to_Sessions`** — the actual definition of "is this device's session still open." **There is no `close_session()`-style function anywhere in this codebase.** A session closes purely as an emergent property: `pair_sessions_events()` sets `evePairEventRowid` on a `New Device`/`Connected`/`Down Reconnected` Event to point at the next `Disconnected`/`Device Down` Event for that MAC, and this view computes `sesStillConnected = 1` exactly when that pairing is still `NULL`. If a session needs to close, the fix is always "make sure the right `Events` row gets inserted" — never a direct `Sessions` mutation (the one exception is `create_new_devices()`'s reconnect-insert, noted below).
-- **`DevicesView`** — adds computed `devIsSleeping`/`devFlapping`/`devStatus` on top of `Devices`. This is what the UI and `insertOnlineHistory()` actually read presence from, not the raw `Devices` table.
+- **`LatestDeviceScan`** (`server/db/db_upgrade.py`) — `Devices` LEFT JOIN'd to the most recent `CurrentScan` row per `(scanMac, scanSourcePlugin)` pair, via `ROW_NUMBER() OVER (PARTITION BY scanMac, scanSourcePlugin ...)`. `update_devices_data_from_scan()` loops over `DISTINCT scanSourcePlugin` and re-queries this view once per plugin: when two plugins report the same device in one cycle, each contribution is evaluated separately, per field, through the authority mechanism below — they are not merged into one row first.
+- **`LatestEventsPerMAC`** — most recent Event per MAC, joined to `Devices` and `CurrentScan`. The "New Connections" query in `insert_events()` uses it to decide whether a device was previously down (→ `Down Reconnected`) or new (→ `Connected`).
+- **`Convert_Events_to_Sessions`** — defines "is this device's session still open." There is no `close_session()` function anywhere in this codebase. A session closes as an emergent property: `pair_sessions_events()` sets `evePairEventRowid` on a `New Device`/`Connected`/`Down Reconnected` Event to point at the next `Disconnected`/`Device Down` Event for that MAC; this view sets `sesStillConnected = 1` exactly when that pairing is `NULL`. To close a session, insert the right `Events` row — never mutate `Sessions` directly (the one exception is `create_new_devices()`'s reconnect-insert, in the call order below).
+- **`DevicesView`** — adds computed `devIsSleeping`/`devFlapping`/`devStatus` on top of `Devices`. The UI and `insertOnlineHistory()` read presence from this, not the raw `Devices` table.
 
-## `process_scan()` call order (`server/scan/session_events.py`) — the order is load-bearing, not incidental
+## `process_scan()` call order (`server/scan/session_events.py`) — order is load-bearing
 
 1. `save_own_device()`, `exclude_ignored_devices()`
-2. `insert_events(db)` — **runs before presence gets updated for this cycle.** Deliberate: the Down/Disconnected/Connected queries need the *previous* cycle's `devPresentLastScan` value to detect a transition (present last cycle but absent now, or vice versa). If this ran after the presence update, every query would see the already-updated value and the edge-triggered design would collapse into either never firing or firing every cycle.
-3. `create_new_devices(db)` — the source comment is explicit: "after create events -> avoid 'connection' event." Brand-new devices get a `New Device` event instead of a `Connected` event, because at step 2 they didn't exist as `Devices` rows for either query to match. Also contains a raw `INSERT INTO Sessions ... sesStillConnected = 1` for devices that already exist but have no currently-open session — the one place outside the `Events`-derived path that writes `Sessions` directly.
+2. `insert_events(db)` — runs before presence updates for this cycle. The Down/Disconnected/Connected queries need the *previous* cycle's `devPresentLastScan` to detect a transition. If this ran after the presence update, every query would see the new value and the edge-triggered design would break — firing never, or every cycle.
+3. `create_new_devices(db)` — runs before presence updates so a brand-new device gets a `New Device` event, not a `Connected` event (it has no `Devices` row yet for step 2's queries to match). Also has a raw `INSERT INTO Sessions ... sesStillConnected = 1` for existing devices with no open session — the one place outside the `Events`-derived path that writes `Sessions` directly.
 4. `update_devices_data_from_scan(db)` — field-level updates for existing devices; see the authority mechanism below.
 5. `update_sync_hub_node`, `update_devLastConnection_from_CurrentScan`
-6. `update_presence_from_CurrentScan(db)` — sets `devPresentLastScan` from bare `CurrentScan` presence *for this cycle* (this becomes the "previous" value step 2 reads on the *next* cycle).
-7. `update_devPresentLastScan_based_on_nics(db)` — NIC/parent-child presence aggregation; can override step 6's result for parent devices.
-8. `update_devPresentLastScan_based_on_force_status(db)` — the user's manual `devForceStatus` override, runs **last**, wins unconditionally over everything above.
+6. `update_presence_from_CurrentScan(db)` — sets `devPresentLastScan` from `CurrentScan` for this cycle (step 2 reads this as "previous" on the *next* cycle).
+7. `update_devPresentLastScan_based_on_nics(db)` — NIC/parent-child presence aggregation; can override step 6 for parent devices.
+8. `update_devPresentLastScan_based_on_force_status(db)` — the user's manual `devForceStatus` override; runs last, wins over everything above.
 9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types`
 10. `pair_sessions_events(db)` — pairs `Events` rows as described above.
-11. `create_sessions_snapshot(db)` — `DELETE FROM Sessions; INSERT INTO Sessions SELECT * FROM Convert_Events_to_Sessions`. This is the point where `Sessions` actually reflects step 10's pairing.
+11. `create_sessions_snapshot(db)` — `DELETE FROM Sessions; INSERT INTO Sessions SELECT * FROM Convert_Events_to_Sessions`. `Sessions` reflects step 10's pairing from here.
 12. `insertOnlineHistory(db)` — dashboard graph rollup.
 13. `skip_repeated_notifications(db)`
-14. `DELETE FROM CurrentScan` — the ephemeral table's entire lifetime is one call to `process_scan()`.
+14. `DELETE FROM CurrentScan` — the table's entire lifetime is one call to `process_scan()`.
 
 ## Field-write authority for scan-derived updates
 
-`update_devices_data_from_scan()` (`server/scan/device_handling.py`) does not blindly overwrite fields from whichever plugin ran most recently. Each trackable field is declared once in `FIELD_SPECS` (`scan_col`, `source_col`, a `priority` list of plugin prefixes, optional `allow_override_if_changed`), and `can_overwrite_field()` uses that plus `get_plugin_authoritative_settings()` (which reads a plugin's own settings for an explicit authority override) to decide, per field per row, whether this plugin's value may replace what's there. The paired `<field>Source` column (`devNameSource`, `devLastIPSource`, etc.) records who currently owns the field. `devMac` itself is never a target of these updates — it's the join key, not a tracked field — so no scan-derived update path can ever alter a device's identity, only its attributes.
+`update_devices_data_from_scan()` (`server/scan/device_handling.py`) does not overwrite fields from whichever plugin ran most recently. Each trackable field is declared once in `FIELD_SPECS` (`scan_col`, `source_col`, a `priority` list of plugin prefixes, optional `allow_override_if_changed`). `can_overwrite_field()` uses that plus `get_plugin_authoritative_settings()` (a plugin's own authority-override setting, if any) to decide, per field per row, whether this plugin's value may replace what's there. The paired `<field>Source` column (`devNameSource`, `devLastIPSource`, etc.) records who currently owns the field. `devMac` is never a target of these updates — it's the join key, not a tracked field — so no scan-derived update can alter a device's identity, only its attributes.
 
-This is the scan-pipeline-local half of a bigger attribution system — see the `database-patterns` skill for `FIELD_SOURCE_MAP` / `server/db/authoritative_handler.py`, the full `*Source` attribution model, and how SQLite triggers consume it for audit logging. Read both if touching anything that writes a `*Source` column.
+This is the scan-pipeline-local half of a bigger attribution system — see `database-patterns` for `FIELD_SOURCE_MAP`/`server/db/authoritative_handler.py`, the full `*Source` model, and the SQLite triggers that consume it for audit logging. Read both before touching anything that writes a `*Source` column.
 
-## Four real gotchas (not hypothetical — all surfaced live during a design review)
+## Gotchas
 
-1. **A "presence" check almost always exists in more than one place.** When adding a per-row signal meaning "don't count this as a live sighting" (e.g. a proposed `scanPresence` column), every query that independently re-derives "is this MAC currently present" from `CurrentScan` has to be updated together — `update_presence_from_CurrentScan()`, the `insert_events()` "New Connections"/"Device Down"/"Disconnected" queries, and the raw `INSERT INTO Sessions` inside `create_new_devices()` all encode that same question separately. Patching one and missing a sibling produces a UI where the device badge, the Events log, and the Sessions timeline each tell a different story for the same device. See `.gemini/internal-docs/PRDs/plugin-import-behavior-controls.md` for the worked example — a `scanPresence = 0` transition that never closed its session because only one of three "is it present" queries had been patched. **Update:** `current_scan_presence_condition()` (`server/scan/presence.py`) now centralizes this for five of those sites — `update_presence_from_CurrentScan()` (both statements), `update_devLastConnection_from_CurrentScan()`, and three of `insert_events()`'s four queries (both `Device Down` variants, `Disconnected`) all call it instead of writing their own `EXISTS (...)`. The remaining two ("New Connections", the raw `Sessions` insert in `create_new_devices()`) still can't use it — they need the actual `scanLastIP`/`scanVendor` *value* off the presence-asserting row via `MIN()`/`GROUP BY`, not just a boolean — so a brand-new presence-adjacent query still has to be checked against both patterns, not assumed to be a bare helper call.
-2. **`CurrentScan` is deleted at the end of every cycle — a per-row flag on it cannot express a decision that needs to survive to a cycle where the row is absent.** Anything that fires specifically *because* a row is missing (`Device Down`, `Disconnected`) cannot read a flag that lived on that now-gone row. If a per-row plugin signal needs to affect behavior beyond the cycle it arrived in, persist it onto the `Devices` row at creation time (e.g. seeding `devAlertDown`/`devAlertEvents` from the row's flag instead of the global `NEWDEV_*` defaults) rather than trying to make the ephemeral table carry it forward.
-3. **`CurrentScan` is not small, and it has an index now — check before assuming otherwise.** Real production users run 10,000+ devices; with the normal one-row-per-contributing-plugin pattern (see `LatestDeviceScan` above), a single cycle's `CurrentScan` is routinely 20,000-50,000+ rows, not the few hundred a homelab install might suggest. `idx_currentscan_scanmac` was added to `server/db/db_upgrade.py:ensure_CurrentScan()` (and mirrored in `server/db/schema/app.sql`) specifically because every `scanMac`-keyed lookup in this file was a full table scan without it — confirmed via `EXPLAIN QUERY PLAN` before the fix. Note `ensure_CurrentScan()` itself (the `DROP TABLE`/`CREATE TABLE`) only runs once, at app startup (`DB.initDB()`, called once from `server/__main__.py`) — don't confuse this with the per-cycle `DELETE FROM CurrentScan` in point 1 above, which clears rows but doesn't touch the table or its index. The index is built once and maintained incrementally, not rebuilt every cycle.
-4. **`server/plugins/sync/sync.py` bypasses this entire pipeline on purpose, twice — a permanent exception, not a bug.** It fires its own direct `INSERT OR IGNORE INTO Events (... 'New Device' ...)` for newly-seen synced devices (hardcoded `evePendingAlertEmail = 1`, no `scanNotificationMode`/quiet awareness), and in `carbon-copy` mode its own raw `Devices` UPSERT via `ON CONFLICT(devMac) DO UPDATE` — both deliberately skipping `create_new_devices()`/`update_devices_data_from_scan()`/`can_overwrite_field()` (`sync.py`'s own comments document this as intentional: "Node is fully authoritative in this mode"). It *is* a normal `mapped_to_table: CurrentScan` plugin for its presence contribution, so `IMPORT_ON`/`scanPresence` apply to it exactly like any other plugin — but its two direct-write paths would silently ignore `scanNotificationMode = 'quiet'` or `scanCreatesDevice = 0` if `sync` ever adopted either. Keep this in mind whenever touching the generic pipeline and assuming every `Events`/`Devices` write went through it — `sync.py` is the one place that doesn't.
-
-**Correction: `app.sql` is not dead code** — an earlier version of this note called it "otherwise-unused." Checked further: `install/production-filesystem/entrypoint.d/25-first-run-db.sh` pipes it straight into `sqlite3` to bootstrap a brand-new database on first install, and `scripts/db_cleanup/regenerate-database.sh` uses it too. `CurrentScan`, `Parameters`, and `Settings` are safe from drift because each has a dedicated `ensure_X()` function (`server/db/db_upgrade.py`) that unconditionally drops and recreates the table on every startup, superseding whatever `app.sql` bootstrapped. `Plugins_Language_Strings` gets the same unconditional drop/recreate, but inside the shared `ensure_plugins_tables()`, not a dedicated function of its own. `AppEvents` gets an equivalent drop/recreate too, via a different mechanism — `AppEvent_obj.__init__()` (`server/workflows/app_events.py`) drops and recreates it on every startup, independent of `db_upgrade.py`. `Devices` has no drop/recreate, but `server/database.py` has 18 explicit `ensure_column()` calls that backfill any column missing from an older `app.sql` snapshot on every startup. **`Events`, `Sessions`, and `Notifications`** — the three tables that genuinely had neither safety net — **now have the same backfill treatment**, per `scan-pipeline-hardening.md` Design §3 (implemented): `ensure_table_columns()` (`server/db/db_upgrade.py`), driven by one Python column-list constant per table (`server/db/schema_columns.py`) that's also diffed against `app.sql` in CI (`test/db/test_schema_drift_guard.py`), so drift between the two is caught rather than silently shipping. `AppEvents`/`Notifications` each also have a *second* schema-definition surface beyond `app.sql` worth knowing about — their own inline `CREATE TABLE IF NOT EXISTS` in `server/workflows/app_events.py`/`server/models/notification_instance.py` respectively — kept in sync via the same drift-check test. Any *new* query added here should still be checked with `EXPLAIN QUERY PLAN` at a realistic row count rather than assumed fine because it "looks like the existing queries" — several of those existing queries were themselves unindexed scans until this was caught. A correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable and be quadratic at this scale.
+1. **A "presence" check exists in more than one place.** A per-row signal meaning "don't count this as a live sighting" (e.g. `scanPresence`) has to reach every query that independently re-derives "is this MAC currently present" from `CurrentScan`. `current_scan_presence_condition()` (`server/scan/presence.py`) centralizes that check for five sites: `update_presence_from_CurrentScan()` (both statements), `update_devLastConnection_from_CurrentScan()`, and three of `insert_events()`'s four queries (both `Device Down` variants, `Disconnected`). Two sites can't use it: the "New Connections" query and the raw `Sessions` insert in `create_new_devices()` need the actual `scanLastIP`/`scanVendor` value off the presence-asserting row via `MIN()`/`GROUP BY`, not just a boolean. Check any new presence-adjacent query against both patterns — a bare helper call isn't always enough.
+2. **`CurrentScan` is deleted at the end of every cycle — a per-row flag on it can't express a decision that needs to survive to a cycle where the row is gone.** Anything that fires because a row is *missing* (`Device Down`, `Disconnected`) can't read a flag that lived on that row. A per-row plugin signal that needs to affect behavior beyond its own cycle has to persist onto the `Devices` row at creation time (e.g. seeding `devAlertDown`/`devAlertEvents` from the row's flag instead of the global `NEWDEV_*` defaults), not ride on the ephemeral table.
+3. **`CurrentScan` is not small, and it's indexed on `scanMac`.** Real production users run 10,000+ devices; with one row per contributing plugin (see `LatestDeviceScan` above), a single cycle's `CurrentScan` is routinely 20,000-50,000+ rows. `idx_currentscan_scanmac` (`server/db/db_upgrade.py:ensure_CurrentScan()`, mirrored in `server/db/schema/app.sql`) covers every `scanMac`-keyed lookup in this file. `ensure_CurrentScan()`'s `DROP TABLE`/`CREATE TABLE` runs once, at app startup (`DB.initDB()`, `server/__main__.py`) — don't confuse this with the per-cycle `DELETE FROM CurrentScan` in point 1, which clears rows but leaves the table and its index in place.
+4. **`server/plugins/sync/sync.py` bypasses this pipeline on purpose, twice — a permanent exception, not a bug.** It fires its own direct `INSERT OR IGNORE INTO Events (... 'New Device' ...)` for newly-seen synced devices (hardcoded `evePendingAlertEmail = 1`, no `scanNotificationMode` awareness), and in `carbon-copy` mode its own raw `Devices` UPSERT via `ON CONFLICT(devMac) DO UPDATE` — both skip `create_new_devices()`/`update_devices_data_from_scan()`/`can_overwrite_field()` (`sync.py`'s own comments: "Node is fully authoritative in this mode"). It's a normal `mapped_to_table: CurrentScan` plugin for its presence contribution, so `IMPORT_ON`/`scanPresence` apply to it like any other plugin — but its two direct-write paths ignore `scanNotificationMode = 'quiet'` or `scanCreatesDevice = 0`. Don't assume every `Events`/`Devices` write goes through the generic pipeline — `sync.py` doesn't.
+5. **A blank/null-equivalent `scanMac` can create a phantom `Devices` row.** `create_new_devices()`'s two creation-path queries filter `scanMac NOT IN (NULL_EQUIVALENTS_SQL)` (`server/scan/device_handling.py`, `const.NULL_EQUIVALENTS_SQL`) as a backstop, because `scanCreatesDevice` defaults to `1` — any plugin reporting a row with no real MAC, without setting `scanCreatesDevice = 0` itself, would otherwise create a `devMac = ''` device, and every other blank-MAC row from every other plugin would then silently write onto it. The filter doesn't replace `scanCreatesDevice = 0` as the correct thing for a plugin to set on such rows; it keeps a MAC-less row inert when some other plugin forgets to. Check any new creation-adjacent query against blank `scanMac` too.
+6. **`app.sql` is not dead code.** `install/production-filesystem/entrypoint.d/25-first-run-db.sh` pipes it into `sqlite3` to bootstrap a brand-new database on first install; `scripts/db_cleanup/regenerate-database.sh` uses it too. `CurrentScan`, `Parameters`, and `Settings` are safe from drift: each has a dedicated `ensure_X()` function (`server/db/db_upgrade.py`) that drops and recreates the table on every startup, superseding whatever `app.sql` bootstrapped. `Plugins_Language_Strings` gets the same treatment inside the shared `ensure_plugins_tables()`. `AppEvents` gets its own drop/recreate via `AppEvent_obj.__init__()` (`server/workflows/app_events.py`), independent of `db_upgrade.py`. `Devices` has no drop/recreate, but `server/database.py` has 18 explicit `ensure_column()` calls that backfill any column missing from an older `app.sql` snapshot on every startup. `Events`, `Sessions`, and `Notifications` get the same backfill via `ensure_table_columns()` (`server/db/db_upgrade.py`), driven by one Python column-list constant per table (`server/db/schema_columns.py`) that's diffed against `app.sql` in CI (`test/db/test_schema_drift_guard.py`). `AppEvents`/`Notifications` each also have a second schema-definition surface — their own inline `CREATE TABLE IF NOT EXISTS` in `server/workflows/app_events.py`/`server/models/notification_instance.py` — kept in sync by the same drift-check test. Check any new query here with `EXPLAIN QUERY PLAN` at a realistic row count rather than assuming it's fine because it resembles an existing one — a correlated subquery re-evaluated per row (an accidental self-join) is the pattern most likely to look reasonable while actually being quadratic at this scale.
 
 ## When to read this vs. other docs/skills
 
-- Writing or reviewing a plugin's `config.json`/data contract → `plugin-development` skill, `docs/PLUGINS_DEV*.md`. This skill is about what happens *after* a plugin's rows land in `CurrentScan`, not the plugin-authoring contract itself.
-- Devices-table write paths, `*Source` attribution, audit/history logging, SQLite triggers → `database-patterns` skill.
-- Actually implementing a change here → read the relevant function in `server/scan/session_events.py` / `server/scan/device_handling.py` directly before trusting this skill's line-number references; they're a map, not a guarantee, and will drift as the code moves.
+- Writing or reviewing a plugin's `config.json`/data contract → `plugin-development`, `docs/PLUGINS_DEV*.md`. This skill covers what happens *after* a plugin's rows land in `CurrentScan`, not the authoring contract.
+- Devices-table write paths, `*Source` attribution, audit/history logging, SQLite triggers → `database-patterns`.
+- Implementing a change here → read the actual function in `server/scan/session_events.py`/`server/scan/device_handling.py` first; this skill's line numbers are a map, not a guarantee, and drift as the code moves.

.github/skills/skill-hygiene/SKILL.md

@@ -0,0 +1,44 @@
+---
+name: netalertx-skill-hygiene
+description: Read before writing or editing any SKILL.md in this repo (.claude/.gemini/.github skills trees). Covers the two standing rules for skill prose - state current behavior only, and prefer plain, short wording - plus the grep sweep to run before calling a skill clean.
+---
+
+# Skill Hygiene
+
+## What a skill is, and isn't
+
+A skill is a live reference for how the system works *now*. It is not a lab notebook, a changelog, or a PRD. History, corrections, and discovery narratives belong in PRDs (`.gemini/internal-docs/PRDs/`, see `prd-writing`) or commit messages — never in a skill body. If a skill needs to change because the code changed, edit it in place; don't leave a trace of what it used to say.
+
+This is a different rule from `prd-writing`'s "leave a visible trail of corrections" — that rule is specifically for PRDs, where the trail is the point. A skill has no reader who benefits from seeing your editing history; it only has readers who need the current fact, stated plainly.
+
+## Rule 1: state current behavior only
+
+Cut anything that narrates the past instead of stating the present:
+
+| Smell | Why it's a problem | Fix |
+|---|---|---|
+| `"Correction: X — an earlier version of this note said Y"` | Narrates an edit, not a fact | Just state the current fact about X |
+| `"(as of 2026-07-04)"` | Hedges instead of committing to the fact | State it as true now; update the line when it stops being true |
+| `"This shipped for real in devParentRelType"` / `"...caught in review"` / `"...caught mid-review"` | Tells the story of finding a bug instead of the rule that prevents it | State the rule; use the real example as a plain parenthetical if it helps, without the origin story |
+| `"Two real examples from this exact process:"` / `"during a past audit"` | Frames the skill as a diary of one session | Turn the anecdote into a timeless illustration, or drop it |
+| `"X now does Y"` / `"X previously did Y"` / `"used to be"` / `"no longer"` when describing *the skill's own past text* | Describes the skill's edit history, not the system | Delete — say what's true now, full stop |
+
+`"no longer"` describing real *system* behavior (e.g. "there is no longer a retry loop here") is fine — that's a fact about the code, not about the skill. The test is: does this sentence describe the codebase, or does it describe a previous version of this document?
+
+## Rule 2: plain words, fewer words
+
+If a shorter or simpler phrasing says the same thing, use it. Cut qualifiers that don't change the meaning ("actually," "really," "genuinely," "in this exact process"). Prefer a plain verb over a nominalization. A dense skill with real information beats a padded one — trim narration and hedging before trimming facts.
+
+## Sweep before calling a skill clean
+
+Run this across `.claude/skills/`, `.gemini/skills/`, `.github/skills/` (or a single file being edited):
+
+```bash
+grep -rniE "as of 202|caught in review|caught mid-review|correction:|correction \(|shipped for real|previously|used to be|no longer|originally|was later|historically|in the past|distilled from|real mistake|it turned out|turns out|discovered that|during a past" .claude/skills/ .gemini/skills/ .github/skills/
+```
+
+Read every hit in context — some are legitimate (a rule instructing PRD authors to write correction trails, or "previously down" describing device state, are not violations). Fix the ones that narrate the skill's own history instead of the system's current behavior.
+
+## Keep the three trees in sync
+
+Most skills exist as three near-identical copies (`.claude/skills/<name>/SKILL.md`, `.gemini/skills/<name>/SKILL.md`, `.github/skills/<name>/SKILL.md` — see `.gemini/skills/skills-index/SKILL.md` for the pairing map). When a hygiene fix changes a skill's body, apply the same fix to all paired copies so they stay identical (frontmatter `name`/`description` may differ per tree's own convention; the body should not). `scripts/check_skill_pairs.py`'s `GROUPS` list only flags when some-but-not-all paired files changed in a diff — it doesn't check the bodies actually match, so a manual diff after editing is still worth it.

.github/skills/skills-overview/SKILL.md

@@ -9,7 +9,7 @@ Three AI assistants are configured for this project, each with their own skill d
 
 - **GitHub Copilot** → `.github/skills/`
 - **Gemini CLI** → `.gemini/skills/`
-- **Claude Code** → `.claude/skills/` (currently mirrors only the 3 highest-value skills below, not the full set)
+- **Claude Code** → `.claude/skills/` (mirrors most, not all, of the shared skills below — see the table's Claude Skill column for which)
 
 Skills with the same purpose exist in more than one, sometimes under different names and with different depth. This index maps them so you can find the richer version when needed. A CI check (`scripts/check_skill_pairs.py`, run as `check-skill-pairs` in `.github/workflows/code-checks.yml`) flags PRs that touch some but not all files in a mirrored group - non-blocking, since some divergence is intentional.
 
@@ -30,7 +30,9 @@ Skills with the same purpose exist in more than one, sometimes under different n
 | Logging | `logging-standards` | `logging-standards` | — | `mylog` levels, message format, what not to log |
 | Scan pipeline internals | `scan-pipeline` | `scan-pipeline` | `scan-pipeline` | `process_scan()` call order and why it's load-bearing, `CurrentScan`/`Events`/`Sessions`/`DevicesView` relationships, how a session actually closes (no `close_session()` exists), and the `FIELD_SPECS` field-write authority mechanism. Complements `database-patterns` (Devices write-path/`*Source` attribution) rather than duplicating it. |
 | Database patterns | `database-patterns` | `database-patterns` | `database-patterns` | Devices table write-path inventory, the `FIELD_SOURCE_MAP`/`*Source` attribution system in `server/db/authoritative_handler.py`, SQLite trigger vs. Python-hook tradeoffs, and event-sourced vs. snapshot audit logging. |
-| PRD writing | `prd-writing` | `prd-writing` | `prd-writing` | Methodology for writing a design doc: challenge the idea, verify every claim against actual code, trace every downstream consumer of a new mechanism, evaluate performance impact against the real schema/indexes, record rejected alternatives and open-issue decisions explicitly, final-check pass before done. Distilled from the `plugin-import-behavior-controls` PRD process, including real mistakes caught mid-review. |
+| PRD writing | `prd-writing` | `prd-writing` | `prd-writing` | Methodology for writing a design doc: challenge the idea, verify every claim against actual code, trace every downstream consumer of a new mechanism, evaluate performance impact against the real schema/indexes, record rejected alternatives and open-issue decisions explicitly, final-check pass before done. |
+| UX/frontend design | `ux-design-patterns` | `ux-design-patterns` | `ux-design-patterns` | Don't invent new UX behavior/visual patterns unless a PRD calls for it - search `front/` for an existing pattern first and reuse it. Priority order for design tradeoffs when several options are reasonable: existing behavior > intuitiveness > information density > usability > utility > uniqueness > industry practices > generic UI. |
+| Skill hygiene | `skill-hygiene` | `skill-hygiene` | `skill-hygiene` | Read before writing/editing any SKILL.md. Two standing rules: state current behavior only (no "Correction:", no "as of <date>", no "caught in review" narration - that trail belongs in PRDs), and prefer plain, short wording. Includes the grep sweep to run before calling a skill clean. |
 
 ---
 

.github/skills/ux-design-patterns/SKILL.md

@@ -0,0 +1,35 @@
+---
+name: netalertx-ux-design-patterns
+description: Read before adding or changing any front/ UI element - a control, layout, button, or interaction pattern. Covers the don't-invent-new-UX-without-a-PRD rule and the priority order for design tradeoffs (existing behavior > intuitiveness > information density > usability > utility > uniqueness > industry practices > generic UI).
+---
+
+# UX / Frontend Design Patterns
+
+## Core principle: reuse before inventing
+
+Don't introduce new UX behavior or visual patterns unless a PRD explicitly calls for it. Before building any new UI element, search the existing frontend for a pattern that already solves this exact need, and reuse its markup/CSS/behavior instead of inventing a new one.
+
+Real, recent example: a presence-page Prev/Next pager was first built with custom `<button class="btn btn-xs">` elements floated in a `.box-header`. The DataTables pagination pattern (`dataTables_wrapper` / `dataTables_paginate` / `ul.pagination` / `li.paginate_button.previous|next`) already existed elsewhere in the app and does the exact same job. The custom version looked visually broken in the actual UI and had to be reimplemented using the existing pattern once that was caught in manual testing - reusing it also picked up dark-mode theming (`front/css/dark-patch.css`'s `.pagination li > a` / `.disabled` rules) for free, which the hand-rolled version didn't have. Grep first: e.g. `grep -rn "pagination\|paginate_button" front/` before adding a "previous/next" control of your own; the same applies to modals, filter inputs, badges, tooltips, tables - anything that already has an established shape somewhere in `front/`.
+
+## Priority order for design decisions
+
+When several options are all locally reasonable, resolve the choice in this order - highest wins on conflict:
+
+1. **Existing behavior** - what does this codebase already do for the same or a similar need? Copy it.
+2. **Intuitiveness** - will a user already familiar with the rest of the app understand this without being told?
+3. **Information density** - does it show what's needed without wasting space or hiding what matters?
+4. **Usability** - is it easy and low-friction to actually use (reachability, click count, error tolerance)?
+5. **Utility** - does it solve the real problem, not just resemble a solution?
+6. **Uniqueness** - is this the app's own distinct answer, used only where nothing generic fits well?
+7. **Industry practices** - conventions users bring in from other apps.
+8. **Generic UI** - a default/framework-provided look, used only when nothing above applies.
+
+This list exists to end debates quickly, not to be argued from the bottom up. The reason it's written down is that #1 is exactly the step that gets skipped under time pressure - checking it first is meant to be fast, not a detour.
+
+## Practical checklist before building a new UI element
+
+1. Grep `front/js/`, `front/css/`, `front/php/` for an existing implementation of the same interaction - a table, a pager, a filter box, a modal, a badge, a status indicator.
+2. If found, reuse its markup and CSS classes directly rather than writing new ones - matching classes inherit theming (dark mode, responsive breakpoints) a new hand-rolled version won't have.
+3. If nothing fits, check whether the PRD driving this change actually calls for new UX. If it doesn't, that's a signal to look harder for an existing pattern, not license to invent one.
+4. If a new pattern is genuinely warranted and the PRD says so, design it using the priority order above, and record the choice and why existing patterns didn't fit in the PRD - the next change will hit the same fork and shouldn't have to re-derive the answer.
+5. Verify visually in a real browser/devcontainer before calling it done. A change that "should work" per the markup isn't confirmed until it's actually rendered - matches the project's general "test the golden path in a browser" rule for frontend changes.

.gitignore

@@ -21,6 +21,14 @@ front/log/*
 /log/*
 .gemini/internal-docs/PRDs/*
 !.gemini/internal-docs/PRDs/.gitkeep
+!.gemini/internal-docs/PRDs/completed
+!.gemini/internal-docs/PRDs/to_review
+.gemini/internal-docs/PRDs/completed/*
+!.gemini/internal-docs/PRDs/completed/.gitkeep
+.gemini/internal-docs/PRDs/to_review/*
+!.gemini/internal-docs/PRDs/to_review/.gitkeep
+.gemini/internal-docs/research/*
+!.gemini/internal-docs/research/.gitkeep
 /log/plugins/*
 front/api/*
 /api/*

CLAUDE.md

@@ -89,3 +89,5 @@ Procedural/how-to knowledge (running tests, resetting the DB, devcontainer manag
 - No inline imports — everything at module top level.
 - Reuse `test/db_test_helpers.py` for DB mocks/fixtures in tests rather than redefining `DummyDB`/`make_db` locally.
 - Keep files under ~500 lines; split rather than grow.
+- Every Python function/method gets a succinct docstring describing its current use and behavior — one or two sentences, not a changelog of what changed or why (that belongs in the commit/PR, not the docstring).
+- Before adding a new key to `front/php/templates/language/en_us.json`, search it for an existing key with the same text/purpose and reuse it — prefer generic `Gen_*` keys over page-scoped names for genuinely generic UI text (e.g. `Gen_Prev`/`Gen_Next`, not `Presence_Page_Prev`). Only the English file needs a new key; other locales fall back to it automatically.

docs/NOTIFICATIONS.md

@@ -42,7 +42,7 @@ Click the **Read more in the docs.** Link at the top of each plugin to get more
 
 ### Plugin-level per-row overrides
 
-A plugin author can also mark individual rows it reports as `quiet` via the `scanNotificationMode` data column, independent of any user-facing setting above - e.g. a bulk inventory import that shouldn't spam notifications for known-offline devices. This is a plugin-authoring concept, not something configured in the UI - see [Data contract](https://docs.netalertx.com/PLUGINS_DEV_DATA_CONTRACT#import-behavior-columns) for the full behavior (when it applies, and how it combines with the **Alert Events**/**Alert Down** device settings above when multiple plugins report the same device).
+A plugin author can also mark individual rows it reports as `quiet` via the `scanNotificationMode` data column, independent of any user-facing setting above - e.g. a bulk inventory import that shouldn't spam notifications for known-offline devices. This is a plugin-authoring concept, not something configured in the UI - see [Plugin Import Behavior](https://docs.netalertx.com/PLUGINS_IMPORT_BEHAVIOR) for the full behavior (when it applies, and how it combines with the **Alert Events**/**Alert Down** device settings above when multiple plugins report the same device).
 
 ## Global settings ⚙
 

docs/PLUGINS_DEV.md

@@ -243,6 +243,7 @@ Check your plugin against these repo-wide conventions before opening a PR (verif
 - **Keep `description` strings short.** They render directly in the Settings UI. Put implementation rationale and design trade-offs in the plugin's README or code comments, not the UI-facing description.
 - **For "one or more instances of the same thing," use the nested array + popup-form settings pattern**, not a fixed hardcoded count (e.g. "primary"/"secondary"). See `rest_import` (`RSTIMPRT`)'s `imports` setting for a working example — it also gives each instance its own sub-settings (URL, credentials, per-instance flags) for free.
 - **Persist plugin state under `dbFolderPath`, config artifacts under `configPath`** — see [Persisting Plugin Data](#persisting-plugin-data-state--config-files) below.
+- **A setting's `dataType` and `default_value` must actually agree.** `dataType: "array"` (or `"object"`) means `default_value` must be a real JSON literal for that shape — `'["default"]'`, not the bare string `"default"`. `setting_value_to_python_type()` (`server/helper.py`) `json.loads()`s the default at runtime; a bare string fails that parse, silently logs a decode error, and returns `[]` instead of your intended default — this shipped for real in `devParentRelType`/`UI_theme`/`UI_TOPOLOGY_ORDER` before being caught. If `elementOptions` already sets `multiple`/`orderable: "false"`, that's a strong signal the setting is actually scalar and `dataType` should be `"string"`, not `"array"`, regardless of what UI widget (`select`, etc.) renders it.
 
 ---
 
@@ -311,7 +312,7 @@ To always map a static value (not read from plugin output):
 
 ### Import Behavior Columns (`scanCreatesDevice`, `scanNotificationMode`, `scanPresence`)
 
-Three optional columns on `CurrentScan` control what happens once a row reaches it — see the [Data contract](PLUGINS_DEV_DATA_CONTRACT.md#import-behavior-columns) for the full contract (allowed values, defaults, downstream effects). All three default to today's behavior if never mapped, so existing plugins need no changes.
+Three optional columns on `CurrentScan` control what happens once a row reaches it — see [Plugin Import Behavior](PLUGINS_IMPORT_BEHAVIOR.md) for the full contract (allowed values, defaults, downstream effects). All three default to today's behavior if never mapped, so existing plugins need no changes.
 
 Most plugins map a single static value for the whole import via `mapped_to_column_data` — e.g. an enrichment-only plugin that should never originate a new device:
 

docs/PLUGINS_DEV_DATA_CONTRACT.md

@@ -159,70 +159,7 @@ As the documentation might become outdated, it's good practice to check the late
 
 ### Import Behavior Columns
 
-Three optional `CurrentScan` columns, all independent of each other, control what happens once a row reaches the table.
-
-| Column | Type | Default | Meaning |
-|---|---|---|---|
-| `scanCreatesDevice` | boolean | `1` | Whether this row can originate a *new* `Devices` entry. `0` lets an enrich-only plugin (e.g. a hostname resolver) update an already-existing device's fields without ever being able to create one. |
-| `scanNotificationMode` | text (`normal` \| `quiet`) | `normal` | Whether this row's notifications are suppressed. `quiet` always suppresses the outbound email/push; whether the `Events` row itself still gets written depends on the event. **Live** (per-cycle aggregate, reclassifying a row changes future events): `New Device`, `Connected`, `Down Reconnected`, `IP Changed` — audit trail always written. `New Device` isn't gated on `scanPresence = 1` like the other three (see flowcharts below). **Frozen** (`devAlertDown`/`devAlertEvents` seeded at device creation, reclassifying later has no retroactive effect): `Device Down`, `Disconnected` — not symmetric. `Disconnected` always writes its `Events` row (`evePendingAlertEmail = 0` when quiet). `Device Down` writes **no row at all** when `devAlertDown = 0`. |
-| `scanPresence` | boolean | `1` | Whether this row asserts the device is *currently online*. `0` means "identity/inventory data, no presence claim" — not "offline". A reservation, a lease record, or a static IPAM entry are typical `0` cases. |
-
-**Missing vs. invalid values — these behave differently, not interchangeably:**
-
-| Column | Column never mapped (missing) | Mapped but sent an unexpected value (invalid) |
-|---|---|---|
-| `scanCreatesDevice` | `1` (schema `DEFAULT`) | `CHECK (scanCreatesDevice IN (0, 1))` — anything else fails the `INSERT` outright, it does not silently fall back to `1` |
-| `scanNotificationMode` | `normal` (schema `DEFAULT`) | No `CHECK` constraint — any string other than the literal `'quiet'` is treated as `normal`, since the SQL only special-cases that exact value |
-| `scanPresence` | `1` (schema `DEFAULT`) | `CHECK (scanPresence IN (0, 1))` — same as `scanCreatesDevice`, invalid values fail the `INSERT`, they don't default |
-
-**Multiple plugins reporting the same MAC in the same scan cycle** (the normal case, not an edge case — see the `scan-pipeline` skill) resolve per column, not uniformly: `scanCreatesDevice` and `scanPresence` are most-permissive-wins (any row saying `1` wins), while `scanNotificationMode` is most-*restrictive*-wins (any row saying `quiet` suppresses the notification, even if a sibling row says `normal`) — erring toward under-notifying rather than spamming.
-
-**Combination matrix** — not every combination is meaningful for every plugin; pick the one that matches what your plugin actually knows:
-
-| `scanCreatesDevice` | `scanPresence` | Meaning |
-|---|---|---|
-| 1 | 1 | Normal discovery (the default) |
-| 1 | 0 | Inventory/identity import — create the device, but don't claim it's online right now |
-| 0 | 1 | Presence-confirming enrichment — never originate a device, but assert presence for one that exists |
-| 0 | 0 | Silent enrichment — never originate a device, no presence claim either |
-
-`scanNotificationMode` is orthogonal to both of the above and can be combined with any row in the table (e.g. inventory import + quiet, for a fully silent bulk import of known-offline devices).
-
-**Decision: does this row create a device?**
-
-```mermaid
-flowchart TD
-    A[Row reaches CurrentScan] --> B{scanMac blank or<br/>null-equivalent?}
-    B -- yes --> Z[Never creates a device]
-    B -- no --> C{Any row this cycle for this<br/>MAC has scanCreatesDevice = 1?}
-    C -- no, all say 0 --> Y[No device created<br/>enrich-only]
-    C -- yes, at least one --> D{Devices row already<br/>exists for this MAC?}
-    D -- yes --> E[No-op - existing device untouched<br/>by this check]
-    D -- no --> F[New Devices row created<br/>+ New Device event]
-```
-
-**Decision: is this event's notification suppressed?**
-
-```mermaid
-flowchart TD
-    A[Event about to fire] --> B{Fired from a row that exists in<br/>CurrentScan this cycle? New Device /<br/>Connected / Down Reconnected / IP Changed}
-    B -- yes --> C{Live aggregate: any CurrentScan row<br/>for this MAC says<br/>scanNotificationMode = quiet?}
-    C -- yes --> S[Suppressed<br/>evePendingAlertEmail = 0]
-    C -- no --> N[Notified<br/>evePendingAlertEmail = 1]
-    B -- no, fired from row ABSENCE<br/>Device Down / Disconnected --> D{Frozen device setting:<br/>devAlertDown / devAlertEvents,<br/>seeded at creation time}
-    D -- off --> S
-    D -- on --> N
-```
-
-**Worked scenarios:**
-
-| Scenario | `scanCreatesDevice` | `scanPresence` | `scanNotificationMode` | `scanMac` | Outcome |
-|---|---|---|---|---|---|
-| Normal discovery (default plugin behavior) | `1` (default) | `1` (default) | `normal` (default) | real MAC | Device created if new, notified normally, presence tracked live. |
-| Enrich-only plugin (e.g. a hostname resolver) | `0` | `1` (default) | `normal` (default) | real MAC | Never originates a device; still updates an existing device's fields via `FIELD_SPECS`. If another plugin reports the same MAC with `scanCreatesDevice = 1`, the device still gets created (most-permissive-wins) — this plugin's `0` doesn't block it. |
-| Bulk inventory import of known-offline devices | `1` | `0` | `quiet` | real MAC | Creates devices without claiming they're online, and without a wave of "New Device" notifications for a large batch import. |
-| Presence-confirming enrichment (e.g. a DHCP lease scanner) | `0` | `1` | `normal` | real MAC | Confirms an *existing* device is online without ever being the plugin that creates it. |
-| Row with no usable device identity (e.g. an object with no routable MAC available) | `0` | irrelevant | irrelevant | blank / null-equivalent | Never creates a device — but not for symmetric reasons. The blank-MAC guard blocks the whole aggregated group by its shared `scanMac` value, regardless of any individual row's `scanCreatesDevice` (even a stray `1` from an unrelated plugin sharing the same blank `scanMac` can't override it). Setting `scanCreatesDevice = 0` here is still correct practice, but on its own is only this row's vote — most-permissive-wins means a sibling row for the same `scanMac` asserting `1` would still win. The blank-MAC guard is what actually guarantees safety regardless of what other contributors do. |
+Three optional `CurrentScan` columns — `scanCreatesDevice`, `scanNotificationMode`, `scanPresence` — control whether a row can create a device, whether it counts as a live presence signal, and whether its notifications are suppressed. Only relevant if your plugin maps to `mapped_to_table: "CurrentScan"`; all three default to today's behavior if never mapped. See **[Plugin Import Behavior](PLUGINS_IMPORT_BEHAVIOR.md)** for the full contract — value tables, precedence rules, decision flowcharts, and worked scenarios.
 
 ## Examples
 
@@ -350,6 +287,7 @@ tail -f /tmp/log/app.log | grep -i "YOURPREFIX\|Plugins_Objects"
 
 ## See Also
 
+- [Plugin Import Behavior](PLUGINS_IMPORT_BEHAVIOR.md) - `scanCreatesDevice`/`scanNotificationMode`/`scanPresence`, for plugins mapping to `CurrentScan`
 - [Plugin Settings System](PLUGINS_DEV_SETTINGS.md) - How to accept user input
 - [Data Sources](PLUGINS_DEV_DATASOURCES.md) - Different data source types
 - [Debugging Plugins](DEBUG_PLUGINS.md) - Troubleshooting plugin issues

docs/PLUGINS_IMPORT_BEHAVIOR.md

@@ -0,0 +1,66 @@
+# Plugin Import Behavior
+
+Three optional `CurrentScan` columns, all independent of each other, control what happens once a row your plugin reports reaches the `CurrentScan` table: whether it can create a device, whether it counts as a live presence signal, and whether its notifications are suppressed. This only matters if your plugin maps to `mapped_to_table: "CurrentScan"` — see the [Data contract](PLUGINS_DEV_DATA_CONTRACT.md) for the base column spec these three sit alongside.
+
+| Column | Type | Default | Meaning |
+|---|---|---|---|
+| `scanCreatesDevice` | boolean | `1` | Whether this row can originate a *new* `Devices` entry. `0` lets an enrich-only plugin (e.g. a hostname resolver) update an already-existing device's fields without ever being able to create one. |
+| `scanNotificationMode` | text (`normal` \| `quiet`) | `normal` | Whether this row's notifications are suppressed. `quiet` always suppresses the outbound email/push; whether the `Events` row itself still gets written depends on the event. **Live** (per-cycle aggregate, reclassifying a row changes future events): `New Device`, `Connected`, `Down Reconnected`, `IP Changed` — audit trail always written. `New Device` isn't gated on `scanPresence = 1` like the other three (see flowcharts below). **Frozen** (`devAlertDown`/`devAlertEvents` seeded at device creation, reclassifying later has no retroactive effect): `Device Down`, `Disconnected` — not symmetric. `Disconnected` always writes its `Events` row (`evePendingAlertEmail = 0` when quiet). `Device Down` writes **no row at all** when `devAlertDown = 0`. |
+| `scanPresence` | boolean | `1` | Whether this row asserts the device is *currently online*. `0` means "identity/inventory data, no presence claim" — not "offline". A reservation, a lease record, or a static IPAM entry are typical `0` cases. |
+
+**Missing vs. invalid values — these behave differently, not interchangeably:**
+
+| Column | Column never mapped (missing) | Mapped but sent an unexpected value (invalid) |
+|---|---|---|
+| `scanCreatesDevice` | `1` (schema `DEFAULT`) | `CHECK (scanCreatesDevice IN (0, 1))` — anything else fails the `INSERT` outright, it does not silently fall back to `1` |
+| `scanNotificationMode` | `normal` (schema `DEFAULT`) | No `CHECK` constraint — any string other than the literal `'quiet'` is treated as `normal`, since the SQL only special-cases that exact value |
+| `scanPresence` | `1` (schema `DEFAULT`) | `CHECK (scanPresence IN (0, 1))` — same as `scanCreatesDevice`, invalid values fail the `INSERT`, they don't default |
+
+**Multiple plugins reporting the same MAC in the same scan cycle** (the normal case, not an edge case — see the `scan-pipeline` skill) resolve per column, not uniformly: `scanCreatesDevice` and `scanPresence` are most-permissive-wins (any row saying `1` wins), while `scanNotificationMode` is most-*restrictive*-wins (any row saying `quiet` suppresses the notification, even if a sibling row says `normal`) — erring toward under-notifying rather than spamming.
+
+**Combination matrix** — not every combination is meaningful for every plugin; pick the one that matches what your plugin actually knows:
+
+| `scanCreatesDevice` | `scanPresence` | Meaning |
+|---|---|---|
+| 1 | 1 | Normal discovery (the default) |
+| 1 | 0 | Inventory/identity import — create the device, but don't claim it's online right now |
+| 0 | 1 | Presence-confirming enrichment — never originate a device, but assert presence for one that exists |
+| 0 | 0 | Silent enrichment — never originate a device, no presence claim either |
+
+`scanNotificationMode` is orthogonal to both of the above and can be combined with any row in the table (e.g. inventory import + quiet, for a fully silent bulk import of known-offline devices).
+
+**Decision: does this row create a device?**
+
+```mermaid
+flowchart TD
+    A[Row reaches CurrentScan] --> B{scanMac blank or<br/>null-equivalent?}
+    B -- yes --> Z[Never creates a device]
+    B -- no --> C{Any row this cycle for this<br/>MAC has scanCreatesDevice = 1?}
+    C -- no, all say 0 --> Y[No device created<br/>enrich-only]
+    C -- yes, at least one --> D{Devices row already<br/>exists for this MAC?}
+    D -- yes --> E[No-op - existing device untouched<br/>by this check]
+    D -- no --> F[New Devices row created<br/>+ New Device event]
+```
+
+**Decision: is this event's notification suppressed?**
+
+```mermaid
+flowchart TD
+    A[Event about to fire] --> B{Fired from a row that exists in<br/>CurrentScan this cycle? New Device /<br/>Connected / Down Reconnected / IP Changed}
+    B -- yes --> C{Live aggregate: any CurrentScan row<br/>for this MAC says<br/>scanNotificationMode = quiet?}
+    C -- yes --> S[Suppressed<br/>evePendingAlertEmail = 0]
+    C -- no --> N[Notified<br/>evePendingAlertEmail = 1]
+    B -- no, fired from row ABSENCE<br/>Device Down / Disconnected --> D{Frozen device setting:<br/>devAlertDown / devAlertEvents,<br/>seeded at creation time}
+    D -- off --> S
+    D -- on --> N
+```
+
+**Worked scenarios:**
+
+| Scenario | `scanCreatesDevice` | `scanPresence` | `scanNotificationMode` | `scanMac` | Outcome |
+|---|---|---|---|---|---|
+| Normal discovery (default plugin behavior) | `1` (default) | `1` (default) | `normal` (default) | real MAC | Device created if new, notified normally, presence tracked live. |
+| Enrich-only plugin (e.g. a hostname resolver) | `0` | `1` (default) | `normal` (default) | real MAC | Never originates a device; still updates an existing device's fields via `FIELD_SPECS`. If another plugin reports the same MAC with `scanCreatesDevice = 1`, the device still gets created (most-permissive-wins) — this plugin's `0` doesn't block it. |
+| Bulk inventory import of known-offline devices | `1` | `0` | `quiet` | real MAC | Creates devices without claiming they're online, and without a wave of "New Device" notifications for a large batch import. |
+| Presence-confirming enrichment (e.g. a DHCP lease scanner) | `0` | `1` | `normal` | real MAC | Confirms an *existing* device is online without ever being the plugin that creates it. |
+| Row with no usable device identity (e.g. an object with no routable MAC available) | `0` | irrelevant | irrelevant | blank / null-equivalent | Never creates a device — but not for symmetric reasons. The blank-MAC guard blocks the whole aggregated group by its shared `scanMac` value, regardless of any individual row's `scanCreatesDevice` (even a stray `1` from an unrelated plugin sharing the same blank `scanMac` can't override it). Setting `scanCreatesDevice = 0` here is still correct practice, but on its own is only this row's vote — most-permissive-wins means a sibling row for the same `scanMac` asserting `1` would still win. The blank-MAC guard is what actually guarantees safety regardless of what other contributors do. |

front/js/network-tree.js

@@ -10,40 +10,59 @@ var hiddenChildren = [];
 var deviceListGlobal = null;
 var myTree;
 
+/**
+ * Build an index of children grouped by parent MAC in a single pass,
+ * so getChildren() doesn't have to rescan the full device list per node.
+ * @param {Array} list - Full device list
+ * @returns {Map<string, Array>} parentMac (lowercased) -> array of child devices
+ */
+function buildChildrenIndex(list)
+{
+    const index = new Map();
+    for (var i in list) {
+      const item = list[i];
+      const parentMac = item.devParentMAC?.toLowerCase() || "";       // null-safe
+      if (parentMac != "") {
+        if (!index.has(parentMac)) index.set(parentMac, []);
+        index.get(parentMac).push(item);
+      }
+    }
+    return index;
+}
+
 /**
  * Recursively get children nodes and build a tree
  * @param {Object} node - Current node
- * @param {Array} list - Full device list
+ * @param {Map} childrenIndex - Index built by buildChildrenIndex()
  * @param {string} path - Path to current node
  * @param {Array} visited - Visited nodes (for cycle detection)
  * @returns {Object} Tree node with children
  */
-function getChildren(node, list, path, visited = [])
+function getChildren(node, childrenIndex, path, visited = [])
 {
     var children = [];
+    const nodeMac = node.devMac?.toLowerCase() || "";               // null-safe
 
     // Check for infinite recursion by seeing if the node has been visited before
-    if (visited.includes(node.devMac.toLowerCase())) {
+    if (visited.includes(nodeMac)) {
         console.error("Infinite recursion detected at node:", node.devMac);
         write_notification("[ERROR] ⚠ Infinite recursion detected. You probably have assigned the Internet node to another children node or to itself. Please open a new issue on GitHub and describe how you did it.", 'interrupt')
         return { error: "Infinite recursion detected", node: node.devMac };
     }
 
     // Add current node to visited list
-    visited.push(node.devMac.toLowerCase());
+    visited.push(nodeMac);
 
-    // Loop through all items to find children of the current node
-    for (var i in list) {
-      const item = list[i];
-      const parentMac = item.devParentMAC?.toLowerCase() || "";       // null-safe
-      const nodeMac = node.devMac?.toLowerCase() || "";               // null-safe
-
-      if (parentMac != "" && parentMac == nodeMac && !hiddenMacs.includes(parentMac)) {
+    // Look up this node's children directly instead of scanning the full list
+    if (!hiddenMacs.includes(nodeMac)) {
+      const candidates = childrenIndex.get(nodeMac) || [];
+      for (var i in candidates) {
+        const item = candidates[i];
 
         visibleNodesCount++;
 
         // Process children recursively, passing a copy of the visited list
-        children.push(getChildren(list[i], list, path + ((path == "") ? "" : '|') + parentMac, visited));
+        children.push(getChildren(item, childrenIndex, path + ((path == "") ? "" : '|') + nodeMac, visited));
       }
     }
 
@@ -100,6 +119,7 @@ function getHierarchy()
   parentNodesCount = 0;
 
   let internetNode = null;
+  const childrenIndex = buildChildrenIndex(deviceListGlobal);
 
   for(i in deviceListGlobal)
   {
@@ -107,7 +127,7 @@ function getHierarchy()
     {
       internetNode = deviceListGlobal[i];
 
-      return (getChildren(internetNode, deviceListGlobal, ''))
+      return (getChildren(internetNode, childrenIndex, ''))
       break;
     }
   }

front/php/templates/language/en_us.json

@@ -351,10 +351,12 @@
     "Gen_LockedDB": "ERROR - DB might be locked - Check F12 Dev tools -> Console or try later.",
     "Gen_NetworkMask": "Network mask",
     "Gen_New": "New",
+    "Gen_Next": "Next",
     "Gen_No_Data": "No data",
     "Gen_Offline": "Offline",
     "Gen_Okay": "Ok",
     "Gen_Online": "Online",
+    "Gen_Prev": "Previous",
     "Gen_Purge": "Purge",
     "Gen_ReadDocs": "Read more in the docs.",
     "Gen_Remove_All": "Remove all",

front/presence.php

@@ -160,6 +160,21 @@
 
               <!-- Calendar -->
               <div id="calendar"></div>
+
+              <!-- Presence pager - same markup/classes DataTables generates for its own Previous/Next
+                   (wrapped in .dataTables_wrapper so the same vendor CSS right-aligns it identically) -->
+              <div class="dataTables_wrapper">
+                <div class="dataTables_paginate">
+                  <ul class="pagination">
+                    <li id="presencePrev" class="paginate_button previous disabled">
+                      <a href="#" onclick="changePresencePage(-1); return false;"><?= lang('Gen_Prev');?></a>
+                    </li>
+                    <li id="presenceNext" class="paginate_button next disabled">
+                      <a href="#" onclick="changePresencePage(1); return false;"><?= lang('Gen_Next');?></a>
+                    </li>
+                  </ul>
+                </div>
+              </div>
             </div>
 
           </div>
@@ -210,6 +225,8 @@ switch ($UI_THEME) {
 <script>
 
   var deviceStatus = 'all';
+  var presencePage = 0;
+  var presenceRequestSeq = 0;
 
   // Read parameters & Initialize components
   main();
@@ -420,6 +437,11 @@ function getDevicesTotals () {
 
 // -----------------------------------------------------------------------------
 function getDevicesPresence (status) {
+  // Reset to the first page whenever a new status is selected (not on Prev/Next)
+  if (status !== deviceStatus) {
+    presencePage = 0;
+  }
+
   // Save status selected
   deviceStatus = status;
 
@@ -473,7 +495,14 @@ function getDevicesPresence (status) {
   // -----------------------------
   // Load Devices as Resources
   // -----------------------------
-  const devicesUrl = `${apiBaseUrl}/devices/by-status?status=${deviceStatus}`;
+  const pageSize = parseInt(getSetting("UI_DEFAULT_PAGE_SIZE"));
+  const devicesUrl = `${apiBaseUrl}/devices/by-status?status=${deviceStatus}`
+    + `&limit=${pageSize + 1}&offset=${presencePage * pageSize}`;
+
+  // Tag this request so a stale response (e.g. a fast Next-then-Prev click
+  // whose first request resolves after the second) can be discarded instead
+  // of overwriting the page the user is actually looking at.
+  const requestSeq = ++presenceRequestSeq;
 
   $.ajax({
     url: devicesUrl,
@@ -482,14 +511,25 @@ function getDevicesPresence (status) {
       "Authorization": `Bearer ${apiToken}`
     },
     success: function(devices) {
+      // A newer request has been fired since this one went out - discard.
+      if (requestSeq !== presenceRequestSeq) {
+        return;
+      }
+
+      // Peek-ahead: requested one extra device to know if there's a next page
+      // without a separate count request.
+      const hasNextPage = devices.length > pageSize;
+      const pageDevices = hasNextPage ? devices.slice(0, pageSize) : devices;
+
       // FullCalendar expects resources array
-      const resources = devices.map(dev => ({
+      const resources = pageDevices.map(dev => ({
         id: dev.devMac,
         title: dev.devName
       }));
 
       $('#calendar').fullCalendar('option', 'resources', resources);
       $('#calendar').fullCalendar('refetchResources');
+      updatePresencePagerControls(hasNextPage);
     }
   });
 
@@ -517,6 +557,25 @@ function getDevicesPresence (status) {
   });
 };
 
+// -----------------------------------------------------------------------------
+// Move the presence resources page by delta (-1 = Prev, 1 = Next) and reload.
+function changePresencePage (delta) {
+  const button = delta < 0 ? $('#presencePrev') : $('#presenceNext');
+  if (button.hasClass('disabled')) {
+    return;
+  }
+  presencePage = Math.max(0, presencePage + delta);
+  getDevicesPresence(deviceStatus);
+}
+
+// -----------------------------------------------------------------------------
+// Enable/disable the Prev/Next pager buttons (DataTables' own convention:
+// a "disabled" class on the <li>, not a disabled attribute on the <a>).
+function updatePresencePagerControls (hasNextPage) {
+  $('#presencePrev').toggleClass('disabled', presencePage === 0);
+  $('#presenceNext').toggleClass('disabled', !hasNextPage);
+}
+
 function hidePresenceSkeleton() {
   hideSpinner();
   $('#presence-skeleton').fadeOut(0, function() { $(this).remove(); });

mkdocs.yml

@@ -110,6 +110,7 @@ nav:
           - Overview: PLUGINS_DEV.md
           - Quick start: PLUGINS_DEV_QUICK_START.md
           - Data contract: PLUGINS_DEV_DATA_CONTRACT.md
+          - Import behavior: PLUGINS_IMPORT_BEHAVIOR.md
           - Settings system: PLUGINS_DEV_SETTINGS.md
           - Data sources: PLUGINS_DEV_DATASOURCES.md
           - UI components: PLUGINS_DEV_UI_COMPONENTS.md

scripts/check_skill_pairs.py

@@ -31,6 +31,8 @@ GROUPS = [
     [".gemini/skills/scan-pipeline/SKILL.md", ".github/skills/scan-pipeline/SKILL.md", ".claude/skills/scan-pipeline/SKILL.md"],
     [".gemini/skills/database-patterns/SKILL.md", ".github/skills/database-patterns/SKILL.md", ".claude/skills/database-patterns/SKILL.md"],
     [".gemini/skills/prd-writing/SKILL.md", ".github/skills/prd-writing/SKILL.md", ".claude/skills/prd-writing/SKILL.md"],
+    [".gemini/skills/ux-design-patterns/SKILL.md", ".github/skills/ux-design-patterns/SKILL.md", ".claude/skills/ux-design-patterns/SKILL.md"],
+    [".gemini/skills/skill-hygiene/SKILL.md", ".github/skills/skill-hygiene/SKILL.md", ".claude/skills/skill-hygiene/SKILL.md"],
     [".gemini/skills/settings/SKILL.md", ".github/skills/settings-management/SKILL.md"],
     [".gemini/skills/mcp-activation/SKILL.md", ".github/skills/mcp-activation/SKILL.md"],
     [".gemini/skills/project-navigation/SKILL.md", ".github/skills/project-navigation/SKILL.md"],

server/api_server/api_server_start.py

@@ -832,6 +832,18 @@ def api_devices_totals_named(payload=None):
             "connected", "down", "favorites", "new", "archived", "all", "my",
             "offline"
         ]}
+    }, {
+        "name": "limit",
+        "in": "query",
+        "required": False,
+        "description": "Max devices to return",
+        "schema": {"type": "integer", "minimum": 1, "maximum": 1000}
+    }, {
+        "name": "offset",
+        "in": "query",
+        "required": False,
+        "description": "Number of devices to skip",
+        "schema": {"type": "integer", "minimum": 0}
     }],
     links={
         "GetOpenPorts": {
@@ -859,8 +871,10 @@ def api_devices_totals_named(payload=None):
 )
 def api_devices_by_status(payload: DeviceListRequest = None):
     status = payload.status if payload else request.args.get("status")
+    limit = payload.limit if payload else request.args.get("limit", type=int)
+    offset = payload.offset if payload else request.args.get("offset", type=int)
     device_handler = DeviceInstance()
-    return jsonify(device_handler.getByStatus(status))
+    return jsonify(device_handler.getByStatus(status, limit, offset))
 
 
 @app.route('/devices/search', methods=['POST'])

server/api_server/graphql_endpoint.py

@@ -12,7 +12,7 @@ from logger import mylog  # noqa: E402 [flake8 lint suppression]
 from const import apiPath, NULL_EQUIVALENTS  # noqa: E402 [flake8 lint suppression]
 from helper import (  # noqa: E402 [flake8 lint suppression]
     is_random_mac,
-    get_number_of_children,
+    count_children_by_parent_mac,
     format_ip_long,
     get_setting_value,
 )
@@ -178,10 +178,11 @@ class Query(ObjectType):
         ]
 
         # Add dynamic fields to each device
+        children_counts = count_children_by_parent_mac(devices_data)
         for device in devices_data:
             device["devIsRandomMac"] = 1 if is_random_mac(device["devMac"]) else 0
-            device["devParentChildrenCount"] = get_number_of_children(
-                device["devMac"], devices_data
+            device["devParentChildrenCount"] = children_counts.get(
+                device["devMac"].strip(), 0
             )
             # Return as string — IPv4 long values can exceed Int's signed 32-bit max (2,147,483,647)
             device["devIpLong"] = str(format_ip_long(device.get("devLastIP", "")))

server/api_server/openapi/schemas.py

@@ -265,6 +265,8 @@ class DeviceListRequest(BaseModel):
             "- offline: Devices not present in the last scan"
         )
     )
+    limit: Optional[int] = Field(None, ge=1, le=1000, description="Max devices to return")
+    offset: Optional[int] = Field(None, ge=0, description="Number of devices to skip")
 
 
 class DeviceListResponse(RootModel):

server/database.py

@@ -19,6 +19,7 @@ from db.db_upgrade import (
     ensure_mac_lowercase_triggers,
     ensure_dangling_parentmac_cleanup_trigger,
     cleanup_existing_dangling_parentmac,
+    cleanup_existing_default_devParentRelType,
     migrate_to_camelcase,
     migrate_timestamps_to_utc,
 )
@@ -248,6 +249,9 @@ class DB:
             # Prevent/repair dangling devParentMAC references left by deleted devices
             cleanup_existing_dangling_parentmac(self.sql)
 
+            # Repair devParentRelType='[]' left by the array/string default_value mismatch
+            cleanup_existing_default_devParentRelType(self.sql)
+
             # Device history table + audit triggers
             ensure_deviceshistory_table(self.sql)
             ensure_deviceshistory_triggers(self.sql)

server/db/db_upgrade.py

@@ -246,6 +246,32 @@ def cleanup_existing_dangling_parentmac(sql) -> bool:
         return False
 
 
+def cleanup_existing_default_devParentRelType(sql) -> bool:
+    """
+    One-time/idempotent cleanup for installations created before this setting's
+    config.json declared "dataType": "array" with a plain-string default_value
+    ("default", not a JSON array) - setting_value_to_python_type() failed to
+    json.loads() that default, logged a decode error on every scan, and wrote
+    the literal string '[]' into devParentRelType for every new device. Repairs
+    rows already stamped with '[]'; the type/default_value mismatch itself is
+    fixed in server/plugins/newdev_template/config.json.
+    """
+    try:
+        sql.execute("""
+            UPDATE Devices
+            SET devParentRelType = 'default'
+            WHERE devParentRelType = '[]'
+        """)
+        if sql.rowcount > 0:
+            mylog("verbose", [f"[db_upgrade] Fixed {sql.rowcount} device(s) with devParentRelType='[]'"])
+
+        return True
+
+    except Exception as e:
+        mylog("none", [f"[db_upgrade] ERROR while cleaning up devParentRelType='[]': {e}"])
+        return False
+
+
 def ensure_views(sql) -> bool:
     """
     Ensures required views exist.

server/helper.py

@@ -650,12 +650,15 @@ def is_random_mac(mac):
 
 
 # -------------------------------------------------------------------------------
-# Helper function to calculate number of children
-def get_number_of_children(mac, devices):
-    # Count children by checking devParentMAC for each device
-    return sum(
-        1 for dev in devices if dev.get("devParentMAC", "").strip() == mac.strip()
-    )
+def count_children_by_parent_mac(devices):
+    """Return {parentMac: childCount} for a device list, keyed by devParentMAC exactly
+    as stored (already lowercased upstream by normalize_mac(), so no case-folding here)."""
+    counts = {}
+    for dev in devices:
+        parent_mac = dev.get("devParentMAC", "").strip()
+        if parent_mac:
+            counts[parent_mac] = counts.get(parent_mac, 0) + 1
+    return counts
 
 
 # -------------------------------------------------------------------------------

server/models/device_instance.py

@@ -451,10 +451,11 @@ class DeviceInstance:
 
         return json_obj
 
-    def getByStatus(self, status=None):
+    def getByStatus(self, status=None, limit=None, offset=None):
         """
-        Return devices filtered by status. Returns all if no status provided.
-        Possible statuses: my, connected, favorites, new, down, archived
+        Return devices filtered by status, ordered by devMac for stable pagination.
+        Returns all matching devices if limit is omitted. Possible statuses:
+        my, connected, favorites, new, down, archived (see get_device_conditions()).
         """
         conn = get_temp_db_connection()
         sql = conn.cursor()
@@ -463,8 +464,17 @@ class DeviceInstance:
         condition = get_device_condition_by_status(status) if status else ""
 
         # Only DevicesView has devFlapping
-        query = f"SELECT * FROM DevicesView {condition}"
-        sql.execute(query)
+        query = f"SELECT * FROM DevicesView {condition} ORDER BY devMac"
+        params = []
+        if limit is not None:
+            query += " LIMIT ? OFFSET ?"
+            params.extend([limit, offset or 0])
+        elif offset is not None:
+            # SQLite's unlimited-limit form - an offset with no limit still
+            # needs a LIMIT clause for OFFSET to take effect.
+            query += " LIMIT -1 OFFSET ?"
+            params.append(offset)
+        sql.execute(query, params)
 
         table_data = []
         for row in sql.fetchall():

server/plugins/newdev_template/config.json

@@ -1459,11 +1459,11 @@
     {
       "function": "devParentRelType",
       "type": {
-        "dataType": "array",
+        "dataType": "string",
         "elements": [
           {
             "elementType": "select",
-            "elementOptions": [{ "orderable": "true"}],
+            "elementOptions": [],
             "transformers": ["deviceRelType"]
           }
         ]

server/plugins/ui_settings/config.json

@@ -685,7 +685,7 @@
     {
       "function": "theme",
       "type": {
-        "dataType": "array",
+        "dataType": "string",
         "elements": [
           {
             "elementType": "select",
@@ -713,7 +713,7 @@
     {
       "function": "TOPOLOGY_ORDER",
       "type": {
-        "dataType": "array",
+        "dataType": "string",
         "elements": [
           {
             "elementType": "select",

test/api_endpoints/test_devices_endpoints.py

@@ -208,6 +208,61 @@ def test_devices_by_status(client, api_token, test_mac):
         delete_dummy(client, api_token, test_mac)
 
 
+def test_devices_by_status_pagination(client, api_token):
+    """limit/offset must page through the same set ORDER BY devMac gives
+    unpaginated, with no gaps or duplicates, and must reject invalid values.
+    Doesn't assume an otherwise-empty DB: reconstructs the full 'my' list from
+    pages and compares it to the unpaginated response instead of asserting
+    exact positions for the 3 dummies.
+    """
+    macs = [f"aa:bb:cc:dd:ee:0{i}" for i in (1, 2, 3)]
+    for mac in macs:
+        create_dummy(client, api_token, mac)
+
+    try:
+        full_resp = client.get("/devices/by-status?status=my", headers=auth_headers(api_token))
+        assert full_resp.status_code == 200
+        full_macs = [d["id"] for d in full_resp.json]
+        assert set(macs).issubset(set(full_macs))
+
+        # Page through the full set in halves and confirm the reassembled
+        # list matches the unpaginated one exactly (no gaps/duplicates).
+        total = len(full_macs)
+        half = (total + 1) // 2
+        page1 = client.get(
+            f"/devices/by-status?status=my&limit={half}&offset=0",
+            headers=auth_headers(api_token),
+        ).json
+        page2 = client.get(
+            f"/devices/by-status?status=my&limit={total - half}&offset={half}",
+            headers=auth_headers(api_token),
+        ).json
+        paged_macs = [d["id"] for d in page1] + [d["id"] for d in page2]
+        assert paged_macs == full_macs
+
+        # offset alone (no limit) must still take effect, not be silently
+        # dropped - regression guard for the LIMIT -1 OFFSET ? fallback.
+        offset_only = client.get(
+            f"/devices/by-status?status=my&offset={half}",
+            headers=auth_headers(api_token),
+        ).json
+        assert [d["id"] for d in offset_only] == full_macs[half:]
+
+        # Invalid limit/offset are rejected, not silently clamped.
+        resp_bad_limit = client.get(
+            "/devices/by-status?status=my&limit=0", headers=auth_headers(api_token)
+        )
+        assert resp_bad_limit.status_code == 422
+
+        resp_bad_offset = client.get(
+            "/devices/by-status?status=my&offset=-1", headers=auth_headers(api_token)
+        )
+        assert resp_bad_offset.status_code == 422
+    finally:
+        for mac in macs:
+            delete_dummy(client, api_token, mac)
+
+
 def test_delete_test_devices(client, api_token):
 
     # Delete by MAC

test/api_endpoints/test_graphq_endpoints.py

@@ -5,7 +5,7 @@ import pytest
 INSTALL_PATH = "/app"
 sys.path.extend([f"{INSTALL_PATH}/server/plugins", f"{INSTALL_PATH}/server"])
 
-from helper import get_setting_value  # noqa: E402 [flake8 lint suppression]
+from helper import get_setting_value, count_children_by_parent_mac  # noqa: E402 [flake8 lint suppression]
 from api_server.api_server_start import app  # noqa: E402 [flake8 lint suppression]
 
 
@@ -79,6 +79,47 @@ def test_graphql_post_devices(client, api_token):
     assert isinstance(data["devices"]["count"], int)
 
 
+def test_graphql_devices_parent_children_count_matches_recount(client, api_token):
+    """devParentChildrenCount for every returned device must match an independent
+    recount of the same response (regression guard for the O(n^2)->O(n) rewrite of
+    resolve_devices()/count_children_by_parent_mac() in graphql_endpoint.py/helper.py).
+
+    Not seeded against a freshly-created fixture pair: table_devices.json (what
+    resolve_devices() reads) is only refreshed by the periodic update_api() loop,
+    not synchronously on a POST /device/<mac> create - a create-then-query test
+    would be flaky against snapshot staleness. Recomputing from the response
+    itself avoids that while still exercising the real resolver wiring.
+    """
+    query = {
+        "query": """
+        {
+            devices {
+                devices {
+                    devMac
+                    devParentMAC
+                    devParentChildrenCount
+                }
+                count
+            }
+        }
+        """
+    }
+    resp = client.post("/graphql", json=query, headers=auth_headers(api_token))
+    assert resp.status_code == 200
+
+    devices = resp.get_json()["data"]["devices"]["devices"]
+    expected_counts = count_children_by_parent_mac(
+        [{"devParentMAC": d["devParentMAC"]} for d in devices]
+    )
+
+    for device in devices:
+        expected = expected_counts.get((device["devMac"] or "").strip(), 0)
+        assert device["devParentChildrenCount"] == expected, (
+            f"devMac={device['devMac']}: expected {expected}, "
+            f"got {device['devParentChildrenCount']}"
+        )
+
+
 # --- SETTINGS TESTS ---
 def test_graphql_post_settings(client, api_token):
     """POST /graphql should return settings data"""

test/db/test_default_devparentreltype_cleanup.py

@@ -0,0 +1,135 @@
+"""
+Unit tests for the devParentRelType='[]' cleanup migration.
+
+Tests verify that:
+- Rows stamped with the literal string '[]' (written before the
+  dataType:"array"/default_value:"default" mismatch was fixed in
+  newdev_template/config.json) are repaired to 'default'.
+- Non-matching values and rows with no match are left untouched.
+- The migration is idempotent.
+- A SQL failure is caught and reported as False, not raised.
+"""
+
+import sys
+import os
+import pytest
+import sqlite3
+import tempfile
+
+INSTALL_PATH = os.getenv('NETALERTX_APP', '/app')
+sys.path.extend([f"{INSTALL_PATH}/server/plugins", f"{INSTALL_PATH}/server"])
+
+from db.db_upgrade import cleanup_existing_default_devParentRelType  # noqa: E402
+
+
+@pytest.fixture
+def temp_db():
+    """Create a temporary database for testing"""
+    fd, db_path = tempfile.mkstemp(suffix='.db')
+    os.close(fd)
+
+    conn = sqlite3.connect(db_path)
+    cursor = conn.cursor()
+
+    cursor.execute("""
+        CREATE TABLE Devices (
+            devMac TEXT PRIMARY KEY COLLATE NOCASE,
+            devParentRelType TEXT
+        )
+    """)
+
+    conn.commit()
+
+    yield cursor, conn
+
+    conn.close()
+    os.unlink(db_path)
+
+
+class TestCleanupExistingDefaultDevParentRelType:
+    """Test suite for the one-time/idempotent data repair migration"""
+
+    def test_cleanup_fixes_matching_rows(self, temp_db):
+        cursor, conn = temp_db
+
+        cursor.execute(
+            "INSERT INTO Devices (devMac, devParentRelType) VALUES (?, ?)",
+            ("aa:bb:cc:dd:ee:01", "[]"),
+        )
+        conn.commit()
+
+        assert cleanup_existing_default_devParentRelType(cursor) is True
+
+        cursor.execute(
+            "SELECT devParentRelType FROM Devices WHERE devMac = ?",
+            ("aa:bb:cc:dd:ee:01",),
+        )
+        assert cursor.fetchone() == ("default",)
+
+    def test_cleanup_preserves_nonmatching_values(self, temp_db):
+        cursor, conn = temp_db
+
+        cursor.execute(
+            "INSERT INTO Devices (devMac, devParentRelType) VALUES (?, ?)",
+            ("aa:bb:cc:dd:ee:02", "nic"),
+        )
+        conn.commit()
+
+        cleanup_existing_default_devParentRelType(cursor)
+
+        cursor.execute(
+            "SELECT devParentRelType FROM Devices WHERE devMac = ?",
+            ("aa:bb:cc:dd:ee:02",),
+        )
+        assert cursor.fetchone() == ("nic",)
+
+    def test_cleanup_no_matching_rows_is_a_noop(self, temp_db):
+        cursor, conn = temp_db
+
+        cursor.execute(
+            "INSERT INTO Devices (devMac, devParentRelType) VALUES (?, ?)",
+            ("aa:bb:cc:dd:ee:03", "default"),
+        )
+        conn.commit()
+
+        assert cleanup_existing_default_devParentRelType(cursor) is True
+
+        cursor.execute(
+            "SELECT devParentRelType FROM Devices WHERE devMac = ?",
+            ("aa:bb:cc:dd:ee:03",),
+        )
+        assert cursor.fetchone() == ("default",)
+
+    def test_cleanup_is_idempotent(self, temp_db):
+        cursor, conn = temp_db
+
+        cursor.execute(
+            "INSERT INTO Devices (devMac, devParentRelType) VALUES (?, ?)",
+            ("aa:bb:cc:dd:ee:04", "[]"),
+        )
+        conn.commit()
+
+        assert cleanup_existing_default_devParentRelType(cursor) is True
+        assert cleanup_existing_default_devParentRelType(cursor) is True
+
+        cursor.execute(
+            "SELECT devParentRelType FROM Devices WHERE devMac = ?",
+            ("aa:bb:cc:dd:ee:04",),
+        )
+        assert cursor.fetchone() == ("default",)
+
+    def test_cleanup_reports_sql_failure_as_false(self):
+        """No devParentRelType column at all -> the UPDATE raises, caught and
+        reported as False rather than propagating the exception."""
+        fd, db_path = tempfile.mkstemp(suffix='.db')
+        os.close(fd)
+        conn = sqlite3.connect(db_path)
+        cursor = conn.cursor()
+        cursor.execute("CREATE TABLE Devices (devMac TEXT PRIMARY KEY)")
+        conn.commit()
+
+        try:
+            assert cleanup_existing_default_devParentRelType(cursor) is False
+        finally:
+            conn.close()
+            os.unlink(db_path)

test/plugins/test_plugin_conventions.py

@@ -75,6 +75,38 @@ def test_config_json_is_valid_json(plugin_name):
         pytest.fail(f'{plugin_name}/config.json is not valid JSON: {e}')
 
 
+@pytest.mark.parametrize('plugin_name', _PLUGIN_NAMES)
+def test_array_object_default_value_parses(plugin_name):
+    """A setting declared `dataType: "array"` or `"object"` must have a
+    `default_value` that actually parses as that type - the same
+    `json.loads()` `setting_value_to_python_type()` (server/helper.py)
+    performs on it at runtime. A bare-string default_value (e.g. "default"
+    instead of the JSON literal '["default"]' or '"default"') fails silently:
+    the JSONDecodeError is caught, logged, and [] is returned - this is the
+    exact bug fixed for devParentRelType/UI_theme/UI_TOPOLOGY_ORDER (all three
+    declared dataType:"array" with a plain-string default_value)."""
+    config = _load_config(plugin_name)
+    for setting in config.get('settings', []):
+        dtype = (setting.get('type') or {}).get('dataType')
+        if dtype not in ('array', 'object'):
+            continue
+        default = setting.get('default_value')
+        if default is None:
+            continue
+        try:
+            json.loads(str(default).replace("'", '"'))
+        except json.JSONDecodeError as e:
+            pytest.fail(
+                f"{plugin_name}: setting '{setting.get('function')}' declares "
+                f"dataType={dtype!r} but default_value={default!r} does not "
+                f"parse as {dtype} ({e}). Either fix default_value to a real "
+                f"{dtype} literal, or change dataType to 'string' if the "
+                f"setting is actually always scalar (check whether "
+                f"elementOptions already says multiple/orderable:false - if "
+                f"so that's a strong signal it should be 'string', not 'array')."
+            )
+
+
 @pytest.mark.parametrize('plugin_name', _PLUGIN_NAMES)
 def test_run_defaults_to_disabled(plugin_name):
     config = _load_config(plugin_name)

test/server/test_helper.py

@@ -0,0 +1,68 @@
+"""
+Unit tests for helper.py's count_children_by_parent_mac().
+
+Tests verify the O(n) bucket-count replacement for the old per-device
+O(n) scan (get_number_of_children) produces identical results.
+"""
+
+import sys
+import os
+
+INSTALL_PATH = os.getenv('NETALERTX_APP', '/app')
+sys.path.extend([f"{INSTALL_PATH}/server/plugins", f"{INSTALL_PATH}/server"])
+
+from helper import count_children_by_parent_mac  # noqa: E402
+
+
+class TestCountChildrenByParentMac:
+    """Test suite for count_children_by_parent_mac()"""
+
+    def test_empty_list_returns_empty_dict(self):
+        assert count_children_by_parent_mac([]) == {}
+
+    def test_parent_with_two_children_and_a_grandchild(self):
+        devices = [
+            {"devMac": "aa:aa:aa:aa:aa:aa", "devParentMAC": ""},
+            {"devMac": "bb:bb:bb:bb:bb:bb", "devParentMAC": "aa:aa:aa:aa:aa:aa"},
+            {"devMac": "cc:cc:cc:cc:cc:cc", "devParentMAC": "aa:aa:aa:aa:aa:aa"},
+            {"devMac": "dd:dd:dd:dd:dd:dd", "devParentMAC": "bb:bb:bb:bb:bb:bb"},
+        ]
+
+        counts = count_children_by_parent_mac(devices)
+
+        assert counts["aa:aa:aa:aa:aa:aa"] == 2
+        assert counts["bb:bb:bb:bb:bb:bb"] == 1
+        # leaf devices are absent from the dict, not present with a 0 value
+        assert "cc:cc:cc:cc:cc:cc" not in counts
+        assert "dd:dd:dd:dd:dd:dd" not in counts
+
+    def test_missing_or_empty_parent_mac_excluded(self):
+        devices = [
+            {"devMac": "aa:aa:aa:aa:aa:aa", "devParentMAC": ""},
+            {"devMac": "bb:bb:bb:bb:bb:bb"},  # devParentMAC key absent entirely
+        ]
+
+        assert count_children_by_parent_mac(devices) == {}
+
+    def test_result_independent_of_input_order(self):
+        devices = [
+            {"devMac": "bb:bb:bb:bb:bb:bb", "devParentMAC": "aa:aa:aa:aa:aa:aa"},
+            {"devMac": "aa:aa:aa:aa:aa:aa", "devParentMAC": ""},
+            {"devMac": "cc:cc:cc:cc:cc:cc", "devParentMAC": "aa:aa:aa:aa:aa:aa"},
+        ]
+
+        assert count_children_by_parent_mac(devices) == count_children_by_parent_mac(
+            list(reversed(devices))
+        )
+
+    def test_lookup_uses_stripped_devmac_like_original(self):
+        # Caller strips devMac before lookup (graphql_endpoint.py); the count
+        # dict's keys must match that stripped form for the lookup to hit.
+        devices = [
+            {"devMac": "aa:aa:aa:aa:aa:aa", "devParentMAC": ""},
+            {"devMac": "bb:bb:bb:bb:bb:bb", "devParentMAC": " aa:aa:aa:aa:aa:aa "},
+        ]
+
+        counts = count_children_by_parent_mac(devices)
+
+        assert counts["aa:aa:aa:aa:aa:aa".strip()] == 1