feat(review): add a performance-regression instruction to the AI review system prompt#2636
Conversation
…ew system prompt REVIEW_SYSTEM_PROMPT exhaustively defined blocker criteria (bug/security/data-loss/build-break/API-contract-break) but never mentioned algorithmic complexity, N+1 queries, or unbounded loop/fanout — a genuinely bad performance change (e.g. a DB query moved inside a loop) was structurally invisible to the reviewer, not merely under-prioritized. Adds a performance-regression category to the blockers taxonomy plus a PERFORMANCE SEVERITY discipline line matching the existing "don't nitpick, flag what matters" tone: a concrete, visible regression is a blocker, a style/micro-optimization preference stays a nit. Closes #2559.
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-02 22:25:26 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 4 non-blocking
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2636 +/- ##
=======================================
Coverage 96.05% 96.05%
=======================================
Files 234 234
Lines 26280 26280
Branches 9531 9531
=======================================
Hits 25244 25244
Misses 425 425
Partials 611 611
🚀 New features to boost your workflow:
|
…nce-regression prompt test Gate review nit: the test only asserted a few substrings, so it would still pass if the unbounded loop/fanout example were accidentally removed from REVIEW_SYSTEM_PROMPT.
Summary
REVIEW_SYSTEM_PROMPT(src/services/ai-review.ts) exhaustively defined blocker criteria — bug/security-hole/data-loss/build-break/API-contract-break — but never mentioned algorithmic complexity, N+1 queries, or unbounded loop/fanout. REES's ~19 analyzers don't target the PR's own new code's runtime performance either (the closed feat(rees): add performance guardrails and regression telemetry #1814 is about REES's OWN service budgets, a different thing). A genuinely bad performance regression (a DB query moved inside a loop) was structurally invisible to the reviewer.blockerstaxonomy bullet, and a newPERFORMANCE SEVERITYdiscipline line matching the existing "don't nitpick, flag what matters" tone — a concrete, visible regression (query/network call moved inside a loop, an unbounded loop/fanout) is a blocker; a style/micro-optimization preference stays a nit.Closes #2559.
Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlint(vianpm run test:ci)npm run typechecknpm run test:coveragelocally — a test asserts the real system prompt sent to the model includes the new blocker category, thePERFORMANCE SEVERITYline, and the severity-discipline phrasing; mutation-tested (reverted the addition, confirmed the test fails).npm run test:workers(vianpm run test:ci)npm run build:mcp(vianpm run test:ci)npm run test:mcp-pack(vianpm run test:ci)npm run ui:openapi:check(vianpm run test:ci) — no OpenAPI-visible changenpm run ui:lint(vianpm run test:ci)npm run ui:typecheck(vianpm run test:ci)npm run ui:build(vianpm run test:ci)npm audit --audit-level=moderateSafety
UI Evidencesection below with screenshots. — N/A, no UI change.Notes