DOCS: skills + SQL cleanup

.github/copilot-instructions.md

@@ -46,6 +46,7 @@ Procedural knowledge lives in `.github/skills/`. Load the appropriate skill when
 | Coding standards | `code-standards` |
 | Devices table write paths, SQLite triggers, audit logging, `*Source` attribution | `database-patterns` |
 | `process_scan()` call order, CurrentScan/Events/Sessions internals, device presence lifecycle | `scan-pipeline` |
+| Write a PRD, design doc, or feature proposal | `prd-writing` |
 
 ## Execution Protocol
 

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

@@ -0,0 +1,59 @@
+---
+name: netalertx-prd-writing
+description: Rigorous PRD-writing methodology for NetAlertX — challenge the idea, verify every claim against actual code, trace every downstream consumer of a mechanism, evaluate performance impact against the schema/indexes that actually exist, record rejected alternatives and open-issue decisions explicitly, and do a dedicated final-check pass before calling it done. Use this when asked to write, draft, or review a PRD, design doc, or feature proposal.
+---
+
+# PRD Writing
+
+## When to use
+
+Triggered by: "write a PRD", "draft a design doc", "spec out this feature", "create a PRD for X". Reserve this for changes where getting the design wrong is expensive to unwind — new cross-cutting mechanisms, schema changes, anything touching multiple subsystems. A one-file bug fix doesn't need this process.
+
+## 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:
+
+- 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.
+
+Both mistakes were plausible, well-written, and wrong. Neither would have survived actually reading the code first.
+
+## Process
+
+1. **Understand the current mechanism by reading the actual code before writing anything.** Cite `file:line` for every claim about current behavior. Delegate to an Explore/general-purpose agent for breadth if the surface area is large, but treat its findings as a starting point to spot-check, not a finished citation — verify anything load-bearing yourself before it goes in the PRD.
+2. **Challenge the idea before designing it.** If the user proposes a solution, ask: is this solving the right problem? Does it conflate unrelated concerns (see axis-separation, next)? Does a similar or previously-rejected mechanism already exist that this would collide with semantically? A naming near-collision with an existing field/concept that has different, incompatible semantics is a signal to stop and check precedence rules, not a coincidence to wave off.
+3. **Identify the independent axes.** A feature request that arrives as "option A and option B" is often two or three orthogonal concerns bundled together — e.g. "should this exist at all," "should it notify," and "should it assert presence" are three separate questions, not one. Cramming them into a single enum/flag produces combinations you can't express later (what if a plugin wants A+C but not B?). Give each axis its own mechanism.
+4. **For every mechanism, trace every downstream consumer — not just the first one you find.** The single highest-value question before calling a design complete: "where else does this exact same check or logic get independently re-derived?" In a codebase without one source of truth for a concept (e.g. "is this record currently active" computed by three different queries in three different files), patching the first occurrence and stopping is the most common way a design ships with a hidden, silent gap. Grep for the pattern, not just the function you already know about.
+5. **Record rejected alternatives with the reasoning, not just the chosen design.** Give it its own subsection (`### Rejected: X`). Without this, a future reader — or your own future self — re-proposes the rejected idea because the "why not" only ever existed in a conversation, not in the document.
+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.
+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.
+    - Does anything render incorrectly as markdown — an unfenced ASCII diagram or code block will collapse into one line under lazy-paragraph-continuation, the same class of bug as a list missing its preceding blank line.
+    - Do any internal anchor links' slugs actually match their headings?
+    - Does the design still cleanly separate its axes, or did a later addition quietly re-conflate two concerns inside what's supposed to be a single-purpose mechanism (the same mistake step 3 exists to catch at the top level can reappear one level down inside an individual mechanism's own value set — e.g. a 3-value enum where two of the values are secretly independent booleans in a trenchcoat).
+11. **Leave a visible trail of corrections instead of silently rewriting.** When a review pass — yours or someone else's — finds something wrong, write "**Correction (caught in review):** ..." inline rather than quietly fixing the earlier text and moving on. This is what makes a PRD trustworthy to a second reader: they can see what was checked and what changed, not just receive a polished final answer with no visible seams.
+
+## Structure to follow
+
+- **Problem** — grounded in specific, cited current behavior, not a general complaint.
+- **Goals / Non-goals** — non-goals should name specific things that sound in-scope but aren't, each with a one-line reason.
+- **Design** — one subsection per independent axis/mechanism (step 3). Include a `### Rejected: X` subsection for any alternative seriously considered (step 5).
+- **Open issues** — each with an explicit recorded decision (step 6), not left dangling.
+- **Affected files** — concrete `file:function:line` references, not bare filenames.
+- **Backward compatibility** — explicit default values and why they preserve current behavior for existing consumers.
+- **Performance impact** — the baseline (what's unindexed/slow *today*, independent of this change), what the change adds that's negligible, what's genuinely new and worth mitigating, and concrete mitigations rather than a vague "should be fine" (step 9).
+- **Docs/skills to update** — anywhere this needs to be reflected outside the code itself (external docs, paired skill files, template files new authors copy from).
+- **Tests** — organized by mechanism, each case naming the real function/query it exercises and the concrete assertion (step 7), plus a manual verification checklist for anything that can't be unit-tested (including an `EXPLAIN QUERY PLAN` check at realistic scale if the Performance impact section found a genuine risk).
+- **(Optional) Execution plan** — phased, referencing the same file/function names used above rather than restating the design in vaguer terms.
+
+## Before starting: check for an existing architecture-reference skill
+
+If a skill already documents the subsystem the feature touches, load it before researching from scratch — don't re-derive call graphs or mechanism details that are already written down. If the feature touches a subsystem with no such skill, and understanding it required significant re-derivation from raw code, that's a signal to write one afterward so the next PRD in that area doesn't start from zero.
+
+## Where to save
+
+`.gemini/internal-docs/PRDs/<kebab-case-name>.md`, unless the user specifies otherwise. Mark the status line (`**Status:** Draft — pending review`) so it's clear this hasn't been approved yet, and keep the author line accurate about who actually made the calls (a design discussion with an assistant is not sole assistant authorship).

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

@@ -51,6 +51,7 @@ This is the scan-pipeline-local half of a bigger attribution system — see the
 
 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.
 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 the otherwise-unused `server/db/schema/app.sql` reference copy) specifically because every `scanMac`-keyed lookup in this file was a full table scan without it — confirmed via `EXPLAIN QUERY PLAN` before the fix. Any *new* query added here should be checked the same way (`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.
 
 ## When to read this vs. other docs/skills
 

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

@@ -30,6 +30,7 @@ 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. |
 
 ---