Skip to content

Commit 4b7bcfd

Browse files
haasonsaashaasonsaas
committed
feat(eval): add error-handling and footgun fixture packs
Add deep review benchmark packs for error-handling regressions and language-specific semantic traps, plus checked-in loader tests that assert the new bug.error-handling.* and bug.lang.* rule IDs.
1 parent 89742b0 commit 4b7bcfd

File tree

3 files changed

+452
-0
lines changed

3 files changed

+452
-0
lines changed

‎eval/fixtures/deep_review_suite/review_depth_error_handling.json‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
{
2+
"name": "review-depth-error-handling",
3+
"author": "diffscope",
4+
"version": "1.0.0",
5+
"description": "Error-handling and panic-safety benchmark pack for deeper live review runs.",
6+
"languages": [
7+
"rust",
8+
"go",
9+
"python",
10+
"typescript"
11+
],
12+
"categories": [
13+
"bug"
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": "rust-unwrap-in-request-handler",
29+
"category": "bug",
30+
"language": "rust",
31+
"difficulty": "Easy",
32+
"diff_content": "diff --git a/src/http.rs b/src/http.rs\nindex 1111111..2222222 100644\n--- a/src/http.rs\n+++ b/src/http.rs\n@@ -1,5 +1,5 @@\n async fn get_user(Query(params): Query<HashMap<String, String>>) -> Result<Json<User>, StatusCode> {\n- let user_id = params.get(\"user_id\").and_then(|value| value.parse::<u64>().ok()).ok_or(StatusCode::BAD_REQUEST)?;\n+ let user_id = params[\"user_id\"].parse::<u64>().unwrap();\n Ok(Json(load_user(user_id).await?))\n }\n",
33+
"expected_findings": [
34+
{
35+
"description": "Request handler unwraps user-controlled input and can panic instead of returning a bad-request error.",
36+
"severity": "Warning",
37+
"category": "Bug",
38+
"file_pattern": "src/http.rs",
39+
"line_hint": 2,
40+
"contains_any": [
41+
"unwrap on user input",
42+
"panic in request handler",
43+
"bad request should be handled gracefully",
44+
"parse::<u64>().unwrap",
45+
"user-controlled input can panic"
46+
],
47+
"rule_id": "bug.error-handling.unwrap-request"
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": "Request paths should return validation failures instead of panicking on malformed user input.",
59+
"source": "deep-review-suite"
60+
},
61+
{
62+
"name": "go-ignored-error-return",
63+
"category": "bug",
64+
"language": "go",
65+
"difficulty": "Easy",
66+
"diff_content": "diff --git a/internal/store/users.go b/internal/store/users.go\nindex 1111111..2222222 100644\n--- a/internal/store/users.go\n+++ b/internal/store/users.go\n@@ -1,6 +1,4 @@\n func DeleteUser(ctx context.Context, db *sql.DB, id int64) error {\n- if _, err := db.ExecContext(ctx, \"DELETE FROM users WHERE id = ?\", id); err != nil {\n- return err\n- }\n+ _, _ = db.ExecContext(ctx, \"DELETE FROM users WHERE id = ?\", id)\n return nil\n }\n",
67+
"expected_findings": [
68+
{
69+
"description": "Database error is explicitly ignored, so delete failures are silently dropped.",
70+
"severity": "Warning",
71+
"category": "Bug",
72+
"file_pattern": "internal/store/users.go",
73+
"line_hint": 2,
74+
"contains_any": [
75+
"ignored error",
76+
"discarded error return",
77+
"db.ExecContext error is dropped",
78+
"silent failure",
79+
"_, _ = db.ExecContext"
80+
],
81+
"rule_id": "bug.error-handling.ignored-error"
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": "Go code should not discard important error returns from database operations.",
93+
"source": "deep-review-suite"
94+
},
95+
{
96+
"name": "python-bare-except-silences-error",
97+
"category": "bug",
98+
"language": "python",
99+
"difficulty": "Easy",
100+
"diff_content": "diff --git a/app/profile.py b/app/profile.py\nindex 1111111..2222222 100644\n--- a/app/profile.py\n+++ b/app/profile.py\n@@ -1,6 +1,7 @@\n def load_profile(path: str) -> dict:\n try:\n return json.loads(Path(path).read_text())\n- except json.JSONDecodeError:\n- return {}\n+ except:\n+ pass\n+ return {}\n",
101+
"expected_findings": [
102+
{
103+
"description": "Bare except swallows every exception, including interrupts and unrelated runtime failures.",
104+
"severity": "Warning",
105+
"category": "Bug",
106+
"file_pattern": "app/profile.py",
107+
"line_hint": 4,
108+
"contains_any": [
109+
"bare except",
110+
"swallows all exceptions",
111+
"except: catches KeyboardInterrupt",
112+
"silences unrelated runtime errors",
113+
"should catch a specific exception"
114+
],
115+
"rule_id": "bug.error-handling.bare-except"
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": "Python exception handlers should avoid bare except blocks that hide real failures.",
127+
"source": "deep-review-suite"
128+
},
129+
{
130+
"name": "ts-unhandled-promise-rejection",
131+
"category": "bug",
132+
"language": "typescript",
133+
"difficulty": "Medium",
134+
"diff_content": "diff --git a/src/jobs.ts b/src/jobs.ts\nindex 1111111..2222222 100644\n--- a/src/jobs.ts\n+++ b/src/jobs.ts\n@@ -1,5 +1,5 @@\n export async function handleJob(job: Job) {\n- await sendWebhook(job);\n+ sendWebhook(job);\n job.status = \"done\";\n }\n",
135+
"expected_findings": [
136+
{
137+
"description": "Async call is started without await or catch, so rejections can become unhandled and the job is marked done too early.",
138+
"severity": "Warning",
139+
"category": "Bug",
140+
"file_pattern": "src/jobs.ts",
141+
"line_hint": 2,
142+
"contains_any": [
143+
"unhandled promise rejection",
144+
"missing await",
145+
"promise is neither awaited nor caught",
146+
"job may be marked done before sendWebhook finishes",
147+
"should await sendWebhook"
148+
],
149+
"rule_id": "bug.error-handling.unhandled-promise"
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": "Promise-returning work should be awaited or handled so failures are observable.",
161+
"source": "deep-review-suite"
162+
}
163+
]
164+
}

‎eval/fixtures/deep_review_suite/review_depth_language_footguns.json‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
{
2+
"name": "review-depth-language-footguns",
3+
"author": "diffscope",
4+
"version": "1.0.0",
5+
"description": "Language-specific semantic footgun benchmark pack for deeper live review runs.",
6+
"languages": [
7+
"go",
8+
"python",
9+
"rust",
10+
"typescript"
11+
],
12+
"categories": [
13+
"bug"
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": "go-nil-interface-comparison",
29+
"category": "bug",
30+
"language": "go",
31+
"difficulty": "Hard",
32+
"diff_content": "diff --git a/internal/validate/error.go b/internal/validate/error.go\nindex 1111111..2222222 100644\n--- a/internal/validate/error.go\n+++ b/internal/validate/error.go\n@@ -1,4 +1,5 @@\n func ValidateConfig(raw []byte) error {\n- return nil\n+ var err *ConfigError = nil\n+ return err\n }\n",
33+
"expected_findings": [
34+
{
35+
"description": "Typed nil pointer is returned as an error interface, so callers can observe a non-nil error even though the concrete pointer is nil.",
36+
"severity": "Warning",
37+
"category": "Bug",
38+
"file_pattern": "internal/validate/error.go",
39+
"line_hint": 3,
40+
"contains_any": [
41+
"typed nil error",
42+
"non-nil interface",
43+
"nil interface gotcha",
44+
"returning *ConfigError as error",
45+
"if err != nil can misfire"
46+
],
47+
"rule_id": "bug.lang.nil-interface"
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": "Go reviewers should catch typed-nil values that become non-nil when stored in interfaces.",
59+
"source": "deep-review-suite"
60+
},
61+
{
62+
"name": "python-mutable-default-arg",
63+
"category": "bug",
64+
"language": "python",
65+
"difficulty": "Easy",
66+
"diff_content": "diff --git a/app/collector.py b/app/collector.py\nindex 1111111..2222222 100644\n--- a/app/collector.py\n+++ b/app/collector.py\n@@ -1,4 +1,3 @@\n-def record_event(event, seen=None):\n- seen = seen or []\n+def record_event(event, seen=[]):\n seen.append(event)\n return seen\n",
67+
"expected_findings": [
68+
{
69+
"description": "Mutable default list is shared across calls and will accumulate state between invocations.",
70+
"severity": "Warning",
71+
"category": "Bug",
72+
"file_pattern": "app/collector.py",
73+
"line_hint": 1,
74+
"contains_any": [
75+
"mutable default argument",
76+
"shared list across calls",
77+
"default [] is reused",
78+
"state leaks between invocations",
79+
"use None and initialize inside"
80+
],
81+
"rule_id": "bug.lang.mutable-default"
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": "Python reviewers should catch mutable default arguments that silently share state.",
93+
"source": "deep-review-suite"
94+
},
95+
{
96+
"name": "rust-lifetime-dangling-ref",
97+
"category": "bug",
98+
"language": "rust",
99+
"difficulty": "Hard",
100+
"diff_content": "diff --git a/src/labels.rs b/src/labels.rs\nindex 1111111..2222222 100644\n--- a/src/labels.rs\n+++ b/src/labels.rs\n@@ -1,3 +1,4 @@\n pub fn default_label() -> &'static str {\n- \"worker\"\n+ let label = String::from(\"worker\");\n+ unsafe { std::mem::transmute::<&str, &'static str>(label.as_str()) }\n }\n",
101+
"expected_findings": [
102+
{
103+
"description": "Unsafe transmute extends the lifetime of a reference to a local String, creating a dangling reference and undefined behavior.",
104+
"severity": "Error",
105+
"category": "Bug",
106+
"file_pattern": "src/labels.rs",
107+
"line_hint": 3,
108+
"contains_any": [
109+
"dangling reference",
110+
"unsafe transmute",
111+
"reference to local string escapes",
112+
"undefined behavior",
113+
"invalid lifetime extension"
114+
],
115+
"rule_id": "bug.lang.dangling-reference"
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": "Rust reviewers should detect unsafe lifetime extension that returns references to freed stack data.",
127+
"source": "deep-review-suite"
128+
},
129+
{
130+
"name": "ts-equality-coercion-trap",
131+
"category": "bug",
132+
"language": "typescript",
133+
"difficulty": "Easy",
134+
"diff_content": "diff --git a/src/filter.ts b/src/filter.ts\nindex 1111111..2222222 100644\n--- a/src/filter.ts\n+++ b/src/filter.ts\n@@ -1,3 +1,3 @@\n export function isZeroish(value: string | number | boolean) {\n- return value === 0;\n+ return value == 0;\n }\n",
135+
"expected_findings": [
136+
{
137+
"description": "Loose equality allows coercion so values like empty strings or false can compare equal to 0 unexpectedly.",
138+
"severity": "Warning",
139+
"category": "Bug",
140+
"file_pattern": "src/filter.ts",
141+
"line_hint": 2,
142+
"contains_any": [
143+
"loose equality",
144+
"type coercion",
145+
"== 0 can match \"\" or false",
146+
"use strict equality",
147+
"=== instead of =="
148+
],
149+
"rule_id": "bug.lang.loose-equality"
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": "TypeScript reviewers should flag loose equality checks that rely on coercive semantics.",
161+
"source": "deep-review-suite"
162+
}
163+
]
164+
}

0 commit comments

Comments
 (0)