You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 491b60b
Browse filesBrowse the repository at this point in the historyBrowse files
Copy file name to clipboardExpand all lines: .claude/skills/prd-writing/SKILL.md
+6-5Lines changed: 6 additions & 5 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -28,15 +28,16 @@ Both are plausible, well-written, and wrong. Reading the code first catches both
28
28
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 intended.
29
29
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).
30
30
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).
31
-
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` for *every* index on the tables involved, not just the first one you find — a column can have both a plain index and a separate expression index (e.g. `idx_eve_mac_date_type ON Events(eveMac, ...)` alongside `idx_eve_lower_mac_date_type ON Events(LOWER(eveMac), ...)`), and missing the second one produces a wrong verdict. 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, built against the *complete* real index set (copy every `CREATE INDEX` for the table, or run it against an actual `app.db`) rather than a hand-picked subset — a partial index set produces a misleading plan in either direction, not just "looks worse than it is." If it comes back unindexed for real, a `GROUP BY` aggregate is the usual fix.
32
-
10.**Do a dedicated final-check pass, out loud, before calling it done.** Re-read the whole document end to end and specifically check:
31
+
9.**Ask explicitly whether this feature/mechanism should be exposed via the API, and give a recommendation, not just flag it as an open question.** This codebase already has three real API surfaces to consider extending rather than inventing a fourth: REST (`server/api_server/*_endpoint.py`), GraphQL (`graphql_endpoint.py`), and a Prometheus `/metrics` endpoint (`prometheus_endpoint.py`). Skipping this question doesn't mean "no API access needed"; it means the answer gets silently decided later by whoever first wants to query the new data externally, usually as its own separate feature request re-litigating a design this PRD already had full context to settle. Not everything needs exposure: purely internal/diagnostic state with no plausible external consumer doesn't, but say so explicitly, with the reason, rather than leaving it unaddressed.
32
+
10. **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` for *every* index on the tables involved, not just the first one you find — a column can have both a plain index and a separate expression index (e.g. `idx_eve_mac_date_type ON Events(eveMac, ...)` alongside `idx_eve_lower_mac_date_type ON Events(LOWER(eveMac), ...)`), and missing the second one produces a wrong verdict. 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, built against the *complete* real index set (copy every `CREATE INDEX` for the table, or run it against an actual `app.db`) rather than a hand-picked subset — a partial index set produces a misleading plan in either direction, not just "looks worse than it is." If it comes back unindexed for real, a `GROUP BY` aggregate is the usual fix.
33
+
11.**Do a dedicated final-check pass, out loud, before calling it done.** Re-read the whole document end to end and specifically check:
33
34
- Did a correction made mid-document 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.
34
35
- 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.
35
36
- 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.
36
37
- Do any internal anchor links' slugs match their headings?
37
38
- 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).
38
-
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.
39
-
12.**When marking a PRD Implemented, ask once whether anything found along the way generalizes beyond this one document** — a tooling trick, a wrong assumption that took real digging to disprove, a review catch that would recur on the next PRD. The PRD's own "Implementation notes" section is the right place for what happened *in this PRD*; a skill or memory update is the right place for anything that would otherwise have to be rediscovered next time. One question, not a mandatory new section — skip it when nothing generalizes, don't force it on every PRD regardless of size.
39
+
12.**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.
40
+
13.**When marking a PRD Implemented, ask once whether anything found along the way generalizes beyond this one document** — a tooling trick, a wrong assumption that took real digging to disprove, a review catch that would recur on the next PRD. The PRD's own "Implementation notes" section is the right place for what happened *in this PRD*; a skill or memory update is the right place for anything that would otherwise have to be rediscovered next time. One question, not a mandatory new section — skip it when nothing generalizes, don't force it on every PRD regardless of size.
40
41
41
42
## Structure to follow
42
43
@@ -46,7 +47,7 @@ Both are plausible, well-written, and wrong. Reading the code first catches both
46
47
-**Open issues** — each with an explicit recorded decision (step 6), not left dangling.
47
48
-**Affected files** — concrete `file:function:line` references, not bare filenames.
48
49
-**Backward compatibility** — explicit default values and why they preserve current behavior for existing consumers.
49
-
-**Performance impact** — the baseline (what's unindexed/slow *today*, independent of this change), what the change adds that's negligible, what's new and worth mitigating, and concrete mitigations rather than a vague "should be fine" (step 9).
50
+
-**Performance impact** — the baseline (what's unindexed/slow *today*, independent of this change), what the change adds that's negligible, what's new and worth mitigating, and concrete mitigations rather than a vague "should be fine" (step 10).
50
51
-**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).
51
52
-**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).
52
53
-**(Optional) Execution plan** — phased, referencing the same file/function names used above rather than restating the design in vaguer terms.
0 commit comments