Skip to content

Commit 45e7c51

Browse files
haasonsaashaasonsaas
committed
feat(eval): add performance fixture pack
Add deep review fixtures for ORM N+1 regressions, unbounded goroutine fan-out, hot-loop cloning, leaked event listeners, and unclosed file handles, plus a checked-in loader test for the new perf.* rule IDs.
1 parent 80f2048 commit 45e7c51

File tree

2 files changed

+269
-0
lines changed

2 files changed

+269
-0
lines changed

‎eval/fixtures/deep_review_suite/review_depth_performance.json‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
{
2+
"name": "review-depth-performance",
3+
"author": "diffscope",
4+
"version": "1.0.0",
5+
"description": "Performance, resource-management, and scalability benchmark pack for deeper live review runs.",
6+
"languages": [
7+
"python",
8+
"go",
9+
"rust",
10+
"typescript"
11+
],
12+
"categories": [
13+
"performance"
14+
],
15+
"thresholds": {
16+
"min_precision": 0.45,
17+
"min_recall": 0.35,
18+
"min_f1": 0.4,
19+
"max_false_positive_rate": 0.45,
20+
"min_weighted_score": 0.45
21+
},
22+
"metadata": {
23+
"purpose": "live-regression-eval",
24+
"family": "review-depth"
25+
},
26+
"fixtures": [
27+
{
28+
"name": "python-n-plus-1-orm-query",
29+
"category": "performance",
30+
"language": "python",
31+
"difficulty": "Medium",
32+
"diff_content": "diff --git a/app/views/orders.py b/app/views/orders.py\nindex 1111111..2222222 100644\n--- a/app/views/orders.py\n+++ b/app/views/orders.py\n@@ -1,6 +1,6 @@\n def list_orders(request):\n- orders = Order.objects.filter(status=\"open\").select_related(\"customer\")\n+ orders = Order.objects.filter(status=\"open\")\n payload = []\n for order in orders:\n payload.append({\"id\": order.id, \"customer\": order.customer.name})\n return JsonResponse({\"orders\": payload})\n",
33+
"expected_findings": [
34+
{
35+
"description": "Removing select_related causes an N+1 query pattern when the loop dereferences order.customer for every row.",
36+
"severity": "Warning",
37+
"category": "Performance",
38+
"file_pattern": "app/views/orders.py",
39+
"line_hint": 2,
40+
"contains_any": [
41+
"n+1 query",
42+
"select_related was removed",
43+
"order.customer in the loop triggers extra queries",
44+
"prefetch the relation",
45+
"orm query per row"
46+
],
47+
"rule_id": "perf.query.n-plus-one"
48+
}
49+
],
50+
"negative_findings": [
51+
{
52+
"description": "Avoid style-only comments.",
53+
"contains": "style"
54+
}
55+
],
56+
"min_total": 1,
57+
"max_total": 8,
58+
"description": "ORM-backed endpoints should preserve eager loading when per-row relation access remains in the response loop.",
59+
"source": "deep-review-suite"
60+
},
61+
{
62+
"name": "go-unbounded-goroutine-spawn",
63+
"category": "performance",
64+
"language": "go",
65+
"difficulty": "Medium",
66+
"diff_content": "diff --git a/internal/jobs/run.go b/internal/jobs/run.go\nindex 1111111..2222222 100644\n--- a/internal/jobs/run.go\n+++ b/internal/jobs/run.go\n@@ -1,5 +1,5 @@\n for _, item := range items {\n- process(item)\n+ go process(item)\n }\n waitForCompletion()\n",
67+
"expected_findings": [
68+
{
69+
"description": "Spawning one goroutine per item without a semaphore or worker pool creates unbounded concurrency and can exhaust memory or CPU under load.",
70+
"severity": "Warning",
71+
"category": "Performance",
72+
"file_pattern": "internal/jobs/run.go",
73+
"line_hint": 2,
74+
"contains_any": [
75+
"unbounded goroutine",
76+
"one goroutine per item",
77+
"missing semaphore or worker pool",
78+
"can overwhelm the runtime",
79+
"needs bounded concurrency"
80+
],
81+
"rule_id": "perf.concurrency.unbounded-goroutines"
82+
}
83+
],
84+
"negative_findings": [
85+
{
86+
"description": "Avoid style-only comments.",
87+
"contains": "style"
88+
}
89+
],
90+
"min_total": 1,
91+
"max_total": 8,
92+
"description": "Batch processing loops should not create unbounded goroutine fan-out without backpressure.",
93+
"source": "deep-review-suite"
94+
},
95+
{
96+
"name": "rust-clone-in-hot-loop",
97+
"category": "performance",
98+
"language": "rust",
99+
"difficulty": "Medium",
100+
"diff_content": "diff --git a/src/metrics.rs b/src/metrics.rs\nindex 1111111..2222222 100644\n--- a/src/metrics.rs\n+++ b/src/metrics.rs\n@@ -2,6 +2,7 @@ pub fn total_name_bytes(records: &[Record]) -> usize {\n let mut total = 0;\n for record in records {\n- total += record.name.len();\n+ let name = record.name.clone();\n+ total += name.len();\n }\n total\n }\n",
101+
"expected_findings": [
102+
{
103+
"description": "Cloning a String inside the loop adds an unnecessary allocation on every iteration of a hot path.",
104+
"severity": "Warning",
105+
"category": "Performance",
106+
"file_pattern": "src/metrics.rs",
107+
"line_hint": 4,
108+
"contains_any": [
109+
"unnecessary clone in hot loop",
110+
"allocates every iteration",
111+
"use a borrowed &str instead",
112+
"clone inside the loop",
113+
"avoid repeated String allocation"
114+
],
115+
"rule_id": "perf.allocation.clone-hot-loop"
116+
}
117+
],
118+
"negative_findings": [
119+
{
120+
"description": "Avoid style-only comments.",
121+
"contains": "style"
122+
}
123+
],
124+
"min_total": 1,
125+
"max_total": 8,
126+
"description": "Hot loops should avoid repeated heap allocations when borrowing is sufficient.",
127+
"source": "deep-review-suite"
128+
},
129+
{
130+
"name": "ts-memory-leak-event-listener",
131+
"category": "performance",
132+
"language": "typescript",
133+
"difficulty": "Medium",
134+
"diff_content": "diff --git a/src/components/Sidebar.tsx b/src/components/Sidebar.tsx\nindex 1111111..2222222 100644\n--- a/src/components/Sidebar.tsx\n+++ b/src/components/Sidebar.tsx\n@@ -3,7 +3,6 @@ export function Sidebar() {\n useEffect(() => {\n window.addEventListener(\"resize\", handleResize);\n- return () => window.removeEventListener(\"resize\", handleResize);\n }, []);\n return <div />;\n }\n",
135+
"expected_findings": [
136+
{
137+
"description": "The resize listener is registered without a cleanup function, so repeated mounts leak listeners and duplicate work.",
138+
"severity": "Warning",
139+
"category": "Performance",
140+
"file_pattern": "src/components/Sidebar.tsx",
141+
"line_hint": 4,
142+
"contains_any": [
143+
"memory leak",
144+
"missing cleanup",
145+
"event listener is never removed",
146+
"useEffect should return a cleanup",
147+
"duplicate listeners on remount"
148+
],
149+
"rule_id": "perf.memory.event-listener-leak"
150+
}
151+
],
152+
"negative_findings": [
153+
{
154+
"description": "Avoid style-only comments.",
155+
"contains": "style"
156+
}
157+
],
158+
"min_total": 1,
159+
"max_total": 8,
160+
"description": "UI effects that register process-wide listeners should clean them up to avoid leaks and duplicate handlers.",
161+
"source": "deep-review-suite"
162+
},
163+
{
164+
"name": "python-file-handle-no-close",
165+
"category": "performance",
166+
"language": "python",
167+
"difficulty": "Easy",
168+
"diff_content": "diff --git a/app/templates.py b/app/templates.py\nindex 1111111..2222222 100644\n--- a/app/templates.py\n+++ b/app/templates.py\n@@ -1,4 +1,4 @@\n def load_template(path: str) -> str:\n- with open(path) as handle:\n- return handle.read()\n+ handle = open(path)\n+ return handle.read()\n",
169+
"expected_findings": [
170+
{
171+
"description": "The file is opened without a context manager or explicit close, so repeated calls can leak file descriptors.",
172+
"severity": "Warning",
173+
"category": "Performance",
174+
"file_pattern": "app/templates.py",
175+
"line_hint": 2,
176+
"contains_any": [
177+
"file handle leak",
178+
"missing close",
179+
"use a with statement",
180+
"descriptor leak",
181+
"open(path) without cleanup"
182+
],
183+
"rule_id": "perf.resource.file-handle-leak"
184+
}
185+
],
186+
"negative_findings": [
187+
{
188+
"description": "Avoid style-only comments.",
189+
"contains": "style"
190+
}
191+
],
192+
"min_total": 1,
193+
"max_total": 8,
194+
"description": "File I/O helpers should use context managers or explicit close paths so descriptors are not leaked under repeated use.",
195+
"source": "deep-review-suite"
196+
}
197+
]
198+
}

‎src/commands/eval/fixtures.rs‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -533,4 +533,75 @@ expect:
533533
Some("design.api.error-type-change")
534534
);
535535
}
536+
537+
#[test]
538+
fn test_checked_in_performance_pack_loads_expected_fixtures() {
539+
let pack_path = std::path::Path::new(env!("CARGO_MANIFEST_DIR"))
540+
.join("eval/fixtures/deep_review_suite/review_depth_performance.json");
541+
542+
let fixtures = load_eval_fixtures_from_path(&pack_path).unwrap();
543+
544+
assert_eq!(fixtures.len(), 5);
545+
let n_plus_one = fixtures
546+
.iter()
547+
.find(|fixture| {
548+
fixture.fixture.name.as_deref()
549+
== Some("review-depth-performance/python-n-plus-1-orm-query")
550+
})
551+
.unwrap();
552+
assert_eq!(
553+
n_plus_one.fixture.expect.must_find[0].rule_id.as_deref(),
554+
Some("perf.query.n-plus-one")
555+
);
556+
557+
let goroutines = fixtures
558+
.iter()
559+
.find(|fixture| {
560+
fixture.fixture.name.as_deref()
561+
== Some("review-depth-performance/go-unbounded-goroutine-spawn")
562+
})
563+
.unwrap();
564+
assert_eq!(
565+
goroutines.fixture.expect.must_find[0].rule_id.as_deref(),
566+
Some("perf.concurrency.unbounded-goroutines")
567+
);
568+
569+
let clone_hot_loop = fixtures
570+
.iter()
571+
.find(|fixture| {
572+
fixture.fixture.name.as_deref()
573+
== Some("review-depth-performance/rust-clone-in-hot-loop")
574+
})
575+
.unwrap();
576+
assert_eq!(
577+
clone_hot_loop.fixture.expect.must_find[0]
578+
.rule_id
579+
.as_deref(),
580+
Some("perf.allocation.clone-hot-loop")
581+
);
582+
583+
let listener_leak = fixtures
584+
.iter()
585+
.find(|fixture| {
586+
fixture.fixture.name.as_deref()
587+
== Some("review-depth-performance/ts-memory-leak-event-listener")
588+
})
589+
.unwrap();
590+
assert_eq!(
591+
listener_leak.fixture.expect.must_find[0].rule_id.as_deref(),
592+
Some("perf.memory.event-listener-leak")
593+
);
594+
595+
let file_handle = fixtures
596+
.iter()
597+
.find(|fixture| {
598+
fixture.fixture.name.as_deref()
599+
== Some("review-depth-performance/python-file-handle-no-close")
600+
})
601+
.unwrap();
602+
assert_eq!(
603+
file_handle.fixture.expect.must_find[0].rule_id.as_deref(),
604+
Some("perf.resource.file-handle-leak")
605+
);
606+
}
536607
}

0 commit comments

Comments
 (0)