Skip to content

Commit 2cbb58a

Browse files
committed
feat(skills): add performance engineering awareness to pb-plan and pb-build prompts
Absorb key insights from performance audit methodology into the system prompts for both planning and building phases. All performance checks are conditional — only applied when tasks touch data access, API endpoints, or hot paths. pb-plan (design phase): - Codebase audit now flags performance-relevant patterns (caching layers, query helpers, pagination utilities, batch processing helpers) - Architecture decisions include brief performance impact assessment for data access and hot path decisions - Architectural Constraints section includes N+1 and over-fetching examples as RFC 2119 constraints - Non-Functional Goals template adds structured fields for database access, caching, API payload, and hot paths - Design template adds 5.8a Performance Impact Assessment table mapping decisions to performance characteristics, risks, and mitigations - Key Principles adds #23: Performance-aware design - Edge cases section adds performance-aware planning guidance pb-build (implementation phase): - Generator workflow adds Performance Sanity Check step (step 9 for BDD+TDD, step 8 for TDD-only) checking for N+1, eager loading, over-fetching - Implementer prompt adds 2i-a. Performance Sanity Check section with specific checks for loops, eager loading, over-fetching, constraints - Evaluator prompt adds performance red flags to code quality checks, new Check E — Performance Sanity, and performance to verdict format - Anti-pattern list adds N+1 queries and over-fetching as known anti-patterns Files changed: - skills/pb-plan/SKILL.md - skills/pb-plan/references/design_template.md - skills/pb-plan/references/tasks_template.md - skills/pb-build/SKILL.md - skills/pb-build/references/implementer_prompt.md - skills/pb-build/references/evaluator_prompt.md
1 parent 6ae437e commit 2cbb58a

6 files changed

Lines changed: 70 additions & 3 deletions

File tree

skills/pb-build/SKILL.md

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,14 @@ The subagent operates as **Generator** — its sole objective is to make tests p
213213
214214
8. **Architecture Conformance Check** — Confirm implementation matches Architecture Decisions.
215215
216-
9. **Scope Check** — Confirm no extra scope beyond the scenario.
216+
9. **Performance Sanity Check (when applicable)** — For tasks touching data access, API endpoints, or hot paths:
217+
- Scan for obvious N+1 query patterns (looping with individual DB calls)
218+
- Verify eager loading or batching is used where the design specifies it
219+
- Check that API responses don't over-fetch beyond what the scenario requires
220+
- If the design includes performance constraints (latency, throughput), verify the implementation doesn't violate them
221+
- Skip this step for tasks that don't touch data access or hot paths
222+
223+
10. **Scope Check** — Confirm no extra scope beyond the scenario.
217224
218225
##### For TDD-Only Tasks (Non-Scenario)
219226
@@ -224,7 +231,8 @@ The subagent operates as **Generator** — its sole objective is to make tests p
224231
5. **REFACTOR** — Clean up. Re-run affected tests.
225232
6. **Runtime Verification (when applicable)** — Run runtime checks.
226233
7. **Architecture Conformance Check** — Verify design conformance.
227-
8. **Scope Check** — Verify scope compliance.
234+
8. **Performance Sanity Check (when applicable)** — Same checks as BDD+TDD tasks above.
235+
9. **Scope Check** — Verify scope compliance.
228236
229237
**The Generator MUST end its output with exactly this signal (no extra text after it):**
230238
@@ -617,6 +625,8 @@ The Evaluator should watch for these common agent mistakes:
617625
| **Speculative features** | Adding caching, validation, notifications to a "save preferences" request | Build only what was asked; add later when needed |
618626
| **Vague success criteria** | "I'll review and improve the code" | "Write test for bug X → make it pass → verify no regressions" |
619627
| **Style drift** | Changing quote style, whitespace, or return logic while adding logging | Match existing code style exactly |
628+
| **N+1 queries** | Looping with individual DB calls instead of batch/eager loading | Use eager loading, batch queries, or prefetch where the design specifies |
629+
| **Over-fetching** | Returning all columns/fields when the consumer only needs a few | Return only required fields; document why wider payload is justified |
620630

621631
---
622632

skills/pb-build/references/evaluator_prompt.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ Analyze the git diff against the task contract.
6666
- Are external dependencies routed through interfaces or abstract classes (DIP)?
6767
- Does each module/class have a single responsibility (SRP)?
6868
- Are the `Architecture Decision Snapshot` constraints from the project preserved?
69+
- Are performance-related design decisions honored (e.g., eager loading, batch queries, field selection)?
6970

7071
3. **Code quality red flags:**
7172
- Hardcoded values that should be configurable
@@ -79,6 +80,11 @@ Analyze the git diff against the task contract.
7980
- Missing `ponytail:` comment on deliberate simplifications
8081
- Code that could be 1 line but isn't
8182
- Missing error handling for external calls
83+
- **Performance red flags** (when the task touches data access or hot paths):
84+
- N+1 query patterns: loops with individual DB calls where batching/eager loading would work
85+
- Over-fetching: returning all columns/fields when the consumer only needs a subset
86+
- Missing eager loading where the design specifies it
87+
- Unbounded result sets without pagination
8288

8389
4. **Dependency integrity:**
8490
- Does this task correctly use outputs from previously completed tasks?
@@ -154,6 +160,20 @@ Apply the ponytail ladder to the diff:
154160

155161
**Output:** Flag any over-engineering found. Each issue should reference the specific ponytail ladder rung violated.
156162

163+
### Check E — Performance Sanity (when applicable)
164+
165+
For tasks that touch data access, API endpoints, or hot paths:
166+
167+
1. **N+1 detection:** Are there loops making individual DB/API calls where batching or eager loading would work?
168+
2. **Eager loading:** Does the implementation use eager loading or joins where the design specifies it?
169+
3. **Over-fetching:** Do API responses return only what the scenario requires, or are extra fields included without justification?
170+
4. **Unbounded results:** Are there queries without pagination or limits that could return unbounded result sets?
171+
5. **Performance constraints:** If the design includes latency/throughput targets, does the implementation obviously violate them?
172+
173+
**Skip** this check for tasks that don't touch data access, API boundaries, or hot paths.
174+
175+
**Output:** List any performance issues found with file:line references.
176+
157177
---
158178

159179
## Verdict
@@ -169,6 +189,7 @@ After completing all three checks, output your verdict in exactly one of these f
169189
- Scope: [Confirmed clean / note any concerns]
170190
- Architecture: [Conformance confirmed / specific violations]
171191
- Code quality: [Clean / issues found and resolved]
192+
- Performance: [Clean / N+1 or over-fetching issues found and resolved / N/A for non-data tasks]
172193
173194
### Live Verification
174195
- [Method used: Playwright MCP / HTTP MCP / CLI]

skills/pb-build/references/implementer_prompt.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,16 @@ Output this analysis enclosed in `<error_analysis>` tags before rewriting code.
163163
- Confirm external dependencies still flow through interfaces or abstract classes when required.
164164
- If you discover the planned architecture no longer fits, stop and raise a Design Change Request instead of improvising a new pattern mid-build.
165165

166+
#### 2i-a. Performance Sanity Check (when applicable)
167+
168+
For tasks touching data access, API endpoints, or hot paths:
169+
170+
1. **N+1 detection:** Scan for loops that make individual DB/API calls where batching or eager loading would work.
171+
2. **Eager loading:** If the design specifies eager loading or joins, verify they are present.
172+
3. **Over-fetching:** Check that API responses return only what the scenario requires.
173+
4. **Performance constraints:** If the design includes latency/throughput targets, verify the implementation doesn't obviously violate them.
174+
5. **Skip** this check for tasks that don't touch data access, API boundaries, or hot paths.
175+
166176
#### 2j. Scope Check
167177

168178
- Confirm implementation matches the task contract and does not include extra scope.

skills/pb-plan/SKILL.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ Subagent rules:
110110
- Existing abstractions (base classes, interfaces, protocols) to extend
111111
- Shared infrastructure (database connections, HTTP clients, cache layers)
112112
- Similar prior implementations that establish patterns to follow
113+
- **Performance-relevant patterns:** Existing caching layers, query optimization helpers, connection pools, pagination utilities, or batch processing helpers
113114
**This audit is mandatory.** List reusable components in `design.md` Section 3.3 and reference them in `tasks.md` task context.
114115

115116
6. **Extract the `Architecture Decision Snapshot`** — this is mandatory whenever the repo has `AGENTS.md` or architecture-oriented docs:
@@ -119,6 +120,7 @@ Subagent rules:
119120
- All external dependencies must be routed **through interfaces or abstract classes** in the plan unless the existing repo explicitly establishes a different seam.
120121
- Reuse existing repo decisions when available; add new decisions only when the requirement creates a genuine gap.
121122
- **Apply the ponytail ladder** when evaluating patterns: prefer stdlib/native solutions over custom implementations. An interface with one implementation is a factory for one product — skip it. A config for a value that never changes is a constant — hardcode it.
123+
- **Performance impact assessment:** For any architectural decision that touches data access, API boundaries, or hot paths, briefly note the expected performance characteristic (e.g., "O(n) queries acceptable for n<100" or "requires lazy loading to avoid N+1"). Do not write full performance specs — just flag the obvious wins and risks.
122124

123125
7. **Audit coding standards and simplification boundaries** — determine which style and maintainability rules the eventual implementation must follow:
124126
- Infer language- and framework-specific standards from `AGENTS.md`, `CLAUDE.md`, and the live codebase. Only apply standards that are relevant to the current repo; do not copy unrelated JavaScript or React rules into Python work.
@@ -253,6 +255,8 @@ Write a **compact** design doc to `specs/<spec-dir>/design.md`. Only include sec
253255
- Database interactions **MUST** use the existing ORM layer; raw SQL queries **MUST NOT** be introduced because they bypass the query builder's injection protection.
254256
- API responses **SHOULD** maintain <200ms p99 latency; if exceeded, the task **MUST** include performance optimization before marking done.
255257
- If an unhandled edge case is encountered, the Builder **MUST** halt and file a DCR; it **MUST NOT** silently fabricate error handling.
258+
- Database queries **MUST NOT** introduce N+1 patterns; the Builder **SHOULD** use eager loading or batch queries where the data access pattern involves related entities.
259+
- API responses **SHOULD** avoid over-fetching; return only fields the consumer needs, or document why a wider payload is justified.
256260

257261
## Behavior Traceability Matrix
258262

@@ -339,6 +343,12 @@ stateDiagram-v2
339343
340344
Read `references/design_template.md` and fill every section fully. Write the result to `specs/<spec-dir>/design.md`.
341345
346+
**Performance awareness for full mode:** When filling the Non-Functional Goals section, explicitly consider:
347+
- **Database access patterns:** Identify whether the feature introduces new queries and whether they risk N+1 or missing indexes.
348+
- **Caching opportunities:** Note if responses or computations are cacheable, and whether existing caching infrastructure can be reused.
349+
- **API payload size:** Flag if the feature returns large datasets and whether pagination or field selection is needed.
350+
- **Hot path identification:** If the feature touches code that runs frequently (request handlers, loops, parsers), note expected throughput requirements.
351+
342352
### Step 5a: Output `tasks.md` — Lightweight Mode (< 50 words)
343353
344354
Write a **scenario-driven task list** to `specs/<spec-dir>/tasks.md`. Tasks are organized by Feature scenarios, not by module. Even in lightweight mode, task IDs must remain in `Task X.Y` format so `pb-build` can track state reliably:
@@ -513,6 +523,7 @@ Please review the design and tasks. When ready, run /pb-build <feature-name> to
513523
20. **Truthfulness over fluency.** If information is missing, state assumptions explicitly instead of fabricating specifics.
514524
21. **Deterministic output quality.** Final docs should be implementation-ready, with no template artifacts left behind.
515525
22. **No speculative architecture.** If a design component cannot be traced to a Feature scenario, remove it. Avoid building what isn't requested.
526+
23. **Performance-aware design.** When architecture decisions touch data access, API boundaries, or hot paths, note the expected performance characteristic. Flag N+1 risks, over-fetching, and missing indexes during the codebase audit. Do not write full performance specs — just identify the obvious wins and risks.
516527

517528
---
518529

@@ -547,3 +558,4 @@ Please review the design and tasks. When ready, run /pb-build <feature-name> to
547558
- **High-variance logic with small example space:** Add property-test planning rather than relying only on hand-written examples.
548559
- **Parser/protocol/unsafe work:** Treat fuzz planning as required unless you can justify why crash-safety risk is absent.
549560
- **Performance-sensitive work:** Add benchmark planning only when the requirement or codebase indicates performance is part of the contract.
561+
- **Performance-aware planning:** When the feature touches data access, API boundaries, or hot paths, flag obvious N+1 risks, over-fetching, and missing indexes during the codebase audit. Include performance constraints in `design.md` §Architectural Constraints when they are part of the contract. Do not write full performance specs for every task — just identify the obvious wins and risks.

skills/pb-plan/references/design_template.md

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,11 @@
7272

7373
> Performance, reliability, security, observability, etc.
7474
75-
- **Performance:** ...
75+
- **Performance:**
76+
- **Database access:** [Identify new queries, N+1 risks, index needs]
77+
- **Caching:** [Cacheable responses, existing cache infrastructure reuse]
78+
- **API payload:** [Over-fetching risks, pagination needs]
79+
- **Hot paths:** [Throughput requirements for frequently-executed code]
7680
- **Reliability:** ...
7781
- **Security:** ...
7882

@@ -242,6 +246,14 @@ graph LR
242246
- **DIP Check:** Identify the seams where dependencies must be inverted.
243247
- **Dependency Injection Plan:** All external dependencies should flow through interfaces or abstract classes unless the repo already has a documented alternative seam.
244248

249+
### 5.8a Performance Impact Assessment
250+
251+
> For architectural decisions that touch data access, API boundaries, or hot paths. Keep it brief — flag obvious wins and risks, not a full performance spec.
252+
253+
| Decision | Performance Characteristic | Risk | Mitigation |
254+
| :--- | :--- | :--- | :--- |
255+
| [e.g., "Use ORM for user queries"] | [e.g., "O(n) for n<100 users"] | [e.g., "N+1 if relations not eager-loaded"] | [e.g., "Use select_related/joinedload"] |
256+
245257
### 5.9 BDD/TDD Strategy
246258

247259
> Describe how this feature will use outside-in development. Define the business-facing Gherkin loop and the supporting TDD loop.

skills/pb-plan/references/tasks_template.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
- **Behavior Preservation Rule:** State whether each task preserves existing behavior or intentionally changes it; validate that with scenario and regression coverage.
2727
- **Simplification Rule:** Apply the ponytail ladder (YAGNI → stdlib → native → existing dep → one-liner → minimum code) at every implementation decision. Mark deferrals with `ponytail:` comments naming the ceiling and upgrade path.
2828
- **Clarity Guardrail:** Avoid planning dense or clever rewrites; where relevant, avoid nested ternary operators in favor of clearer branching.
29+
- **Performance Guardrail:** For tasks touching data access or hot paths, the task context should note expected query patterns and any N+1 or over-fetching risks identified during planning.
2930
- **Phase 1: BDD Harness & Scaffolding** — Feature files, runner setup, task skeletons
3031
- **Phase 2: Scenario Implementation** — Primary behavior implemented via TDD
3132
- **Phase 3: Integration & Features** — Connecting pieces, end-to-end
@@ -186,3 +187,4 @@
186187
6. [ ] **Runtime-Evidenced (when applicable):** Runtime logs and health/probe results are captured, or `N/A` is explicitly justified.
187188
7. [ ] **Behavior-Preserved or Documented:** The task confirms behavior preservation or documents the intentional behavior change.
188189
8. [ ] **Simplified Responsibly:** Cleanup stayed within the planned scope and improved readability rather than introducing clever compaction.
190+
9. [ ] **Performance-Sound (when applicable):** No obvious N+1 queries, over-fetching, or missing eager loading for tasks touching data access or hot paths; or `N/A` is explicitly justified.

0 commit comments

Comments
 (0)