Skip to content

Commit dc2a7df

Browse files
committed
feat: restructure review challenge flow
1 parent 97e2b33 commit dc2a7df

127 files changed

Lines changed: 4703 additions & 2527 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

challenges/README.md

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,32 @@ Current challenge types:
77
- `review/`: review an AI-generated or agent-generated patch and decide whether it can be merged. V0 currently ships 20 review challenges.
88
- `task/`: complete a real engineering task in a starter repository. This will be added after the runner and test harness are ready.
99

10-
Challenge assets should be small, source-traceable, and reproducible. When a challenge is adapted from an upstream open source issue or benchmark, its source must be listed in the challenge README and metadata.
10+
Challenge assets should be small, source-traceable, and reproducible. When a challenge is adapted from an upstream open source issue or benchmark, its source must be listed in the challenge metadata.
11+
12+
## Review challenge assets
1113

1214
Each review challenge contains:
1315

14-
- `metadata.json`
15-
- `README.zh.md`
16-
- `README.en.md`
17-
- `ai-pr.diff`
18-
- `expected-findings.json`
19-
- `rubric.md`
16+
- `metadata.json` — challenge definition. Pre-submit content lives in `title` / `summary` / `tags` / `pr` / `background`; post-submit content lives in `analysis` / `behaviorChecks` / `learningGoal` / `source.references`.
17+
- `ai-pr.diff` — the patch under review, presented as a confident AI-authored PR.
18+
- `src-*.py` / `src-*.ts` / ... — pre-patch source excerpt(s): the minimal sufficient context.
19+
- `expected-findings.json` — answer key: `canMerge`, required/optional findings with `matchTerms` for local scoring. Spoiler file, hidden from the challenge file browser.
20+
- `rubric.md` — structured scoring rubric. Spoiler file.
21+
- `README.zh.md` / `README.en.md` — GitHub-facing brief. Mirrors the pre-submit surface; answers are only pointed to, never inlined.
22+
23+
## The answer distribution is part of the design
24+
25+
Not every challenge is a trap. Some patches are adapted from the correct upstream fix and the right call is to approve them. The pre-submit surface of a mergeable challenge must be indistinguishable from a trap challenge — reviewers earn points by calibrated judgment, not by always answering "request changes".
26+
27+
## Authoring self-check (every challenge must pass all of these)
28+
29+
1. **No spoilers before submit.** Read the full pre-submit surface (title, summary, tags, PR description, background, README, comments inside the diff). If a competent reader can infer the flaw — or infer that there is no flaw — from that surface alone, the challenge fails. The PR speaks only in the author's confident voice.
30+
2. **Minimal sufficient context.** Every required finding must be derivable from the challenge files alone (diff + source excerpt + neutral background). No reliance on knowledge outside the platform. If discovering the flaw requires seeing code we did not include, include it.
31+
3. **A senior engineer must have to think.** Looking only at the PR (not at any hints), reaching the right conclusion should take genuine analysis, not pattern-matching a famous CVE from the library name.
32+
4. **The answer cannot be copied from the prompt.** No pre-submit text may double as an acceptable answer.
33+
5. **Tests inside the diff are part of the trap or part of the evidence.** Trap patches carry tests that look relevant but do not cover the risk; correct patches carry tests a reviewer can legitimately trust. Say which in `analysis`.
34+
6. **Upstream traceability.** `source` must link the real issue/PR/advisory, and `reviewTarget.kind` must state whether the diff is an adapted plausible-but-incorrect patch or an adapted correct fix.
35+
36+
## Using AI while solving is allowed
37+
38+
The platform assumes reviewers may use any AI tool — the trained skill is owning the judgment: giving evidence (line numbers, counterexample inputs, impact chains) and being accountable for the merge decision. Challenges must therefore reward specific evidence rather than generic vulnerability vocabulary, and reference answers are revealed only after a submission.
Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,28 @@
1-
# Review 001: Can this AI fix be merged? Reviewing a SymPy Point2D regression
1+
# Review 001: SymPy Point: fix spurious coordinate rejection under evaluate(False)
22

3-
Review an AI patch for a real SymPy issue: it fixes an evaluate(False) error but may let invalid imaginary coordinates through.
3+
You are a reviewer on this repository. An AI agent submitted the PR below. CI is green and the decision is yours.
44

5-
Your task is to review the adapted AI patch, decide whether it can be merged, and write actionable findings.
5+
## PR description (from the author)
66

7-
## Sources
7+
> **Fix Point raising 'Imaginary coordinates are not permitted' with evaluate(False)**
8+
>
9+
> Parsing Point2D(Integer(1), Integer(2)) inside `with evaluate(False)` raises 'Imaginary coordinates are not permitted' (issue #22684): with automatic evaluation off, im(1) stays as an unevaluated im(1) expression instead of simplifying to 0, so the truthiness test in the coordinate validation treats perfectly real coordinates as having a non-zero imaginary part.
10+
>
11+
> This PR runs the imaginary-coordinate validation only when evaluation is enabled — the check relies on im() being evaluated, so it cannot give a reliable verdict while evaluation is off.
12+
>
13+
> Added test_issue_22684 reproducing the original failure and verifying Point(1, 2) constructs fine under evaluate(False). All existing tests pass.
814
9-
- 原始 Issue: <https://github.com/sympy/sympy/issues/22684>
10-
- 上游正确 PR: <https://github.com/sympy/sympy/pull/22714>
11-
- PatchDiff 论文: <https://arxiv.org/abs/2503.15223>
15+
## What to review
1216

13-
The patch in `ai-pr.diff` is an AgentCode adapted plausible-but-incorrect patch for review training, not the upstream maintainer fix.
17+
Read:
1418

15-
## Context
19+
- `ai-pr.diff` — the patch under review
20+
- `src-point.py` — pre-patch excerpt of Point.__new__ (minimal sufficient context)
1621

17-
- Point / Point2D 是 SymPy 的几何点对象,坐标必须是合法 SymPy 表达式,并且不能是明确的虚数坐标。
18-
- 真实 bug 是 evaluate(False) 环境下普通坐标也可能被旧逻辑误判,从而抛出 Imaginary coordinates are not permitted。
19-
- AI 补丁把整个虚数坐标检查挂到 evaluate 条件后面,容易把“误拒普通坐标”和“拒绝明确虚数坐标”混为一谈。
20-
21-
## Review Format
22+
Then submit your review:
2223

2324
```text
24-
Can merge? Yes / No
25+
Can merge? Yes / No / Need more info
2526
2627
Finding 1:
2728
- Severity:
@@ -30,13 +31,18 @@ Finding 1:
3031
- Suggested fix:
3132
3233
Testing:
33-
- Missing regression or boundary tests:
34+
- What does the new test actually prove? What is missing?
3435
```
3536

36-
## Rubric Focus
37+
## Background
38+
39+
- Point / Point2D are the geometric point objects in SymPy. The constructor validates its inputs; one documented contract is that explicitly imaginary coordinates (e.g. Point(I, 2)) are rejected with a ValueError.
40+
- evaluate(False) globally disables automatic evaluation: calls like im(1) stay in unevaluated form instead of simplifying to 0. SymPy property queries (such as is_zero) use three-valued logic: True / False / None (unknown).
41+
42+
## Answers and analysis
43+
44+
Reference answers live in `expected-findings.json` and `rubric.md` (spoilers). On the website they are revealed after you submit a review.
45+
46+
## Source
3747

38-
- Correct merge decision.
39-
- Core risk: The patch disables the imaginary-coordinate safety check whenever evaluate is false.
40-
- The intended behavior boundary.
41-
- Missing negative or boundary tests.
42-
- Actionable repair direction.
48+
Adapted from a real engineering issue (the `ai-pr.diff` you review is an AgentCode training adaptation, not the upstream fix). Upstream links are in the `source` field of `metadata.json`; read them after attempting the challenge.
Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,28 @@
1-
# Review 001:这个 AI 修复能合并吗?SymPy Point2D 回归审查
1+
# Review 001:SymPy Point:修复 evaluate(False) 下的坐标校验误报
22

3-
你正在审核一个 AI agent 生成的补丁。补丁声称修复 SymPy 中 Point2D 在 evaluate(False) 下误报 Imaginary coordinates are not permitted 的问题
3+
你是这个仓库的 reviewer。一个 AI agent 提交了下面这个 PR,CI 全绿,等待你的结论
44

5-
你的任务不是写代码,而是判断这个 AI PR 是否可以合并。如果不能合并,需要指出具体风险、影响和建议修复方向。
5+
## PR 描述(作者提供)
66

7-
## 题目来源
8-
9-
本题来自真实工程问题,并改编为 AgentCode Review Mode 题目:
10-
11-
- 原始 Issue:<https://github.com/sympy/sympy/issues/22684>
12-
- 上游正确 PR:<https://github.com/sympy/sympy/pull/22714>
13-
- PatchDiff 论文:<https://arxiv.org/abs/2503.15223>
14-
15-
注意:你要审核的是 `ai-pr.diff` 中的改编 AI 补丁,不是上游最终合并的正确修复。
16-
17-
## 背景
18-
19-
- Point / Point2D 是 SymPy 的几何点对象,坐标必须是合法 SymPy 表达式,并且不能是明确的虚数坐标。
20-
- 真实 bug 是 evaluate(False) 环境下普通坐标也可能被旧逻辑误判,从而抛出 Imaginary coordinates are not permitted。
21-
- AI 补丁把整个虚数坐标检查挂到 evaluate 条件后面,容易把“误拒普通坐标”和“拒绝明确虚数坐标”混为一谈。
7+
> **Fix Point raising 'Imaginary coordinates are not permitted' with evaluate(False)**
8+
>
9+
> 在 with evaluate(False) 下解析 Point2D(Integer(1), Integer(2)) 会抛出 Imaginary coordinates are not permitted(issue #22684):关闭自动求值后 im(1) 不会化简成 0,而是保持未求值的 im(1) 表达式,在坐标校验的真值判断里被当成了非零虚部,普通实数坐标因此根本无法构造。
10+
>
11+
> 这个 PR 让虚数坐标校验只在 evaluate 开启时执行——这条校验依赖 im() 的求值结果,在求值被关闭时它给不出可靠结论。
12+
>
13+
> 新增 test_issue_22684 复现原始报错场景,验证 evaluate(False) 下 Point(1, 2) 能正常创建。现有测试全部通过。
2214
2315
## 你需要审核
2416

2517
请阅读:
2618

27-
- `ai-pr.diff`
19+
- `ai-pr.diff` — 待审核的补丁
20+
- `src-point.py` — 补丁前 Point.__new__ 的源码节选(最小充分上下文)
2821

2922
然后提交 review 结论:
3023

3124
```text
32-
Can merge? Yes / No
25+
Can merge? Yes / No / Need more info
3326
3427
Finding 1:
3528
- Severity:
@@ -38,13 +31,18 @@ Finding 1:
3831
- Suggested fix:
3932
4033
Testing:
41-
- Missing regression or boundary tests:
34+
- 新增测试证明了什么?还缺什么?
4235
```
4336

44-
## 评分重点
37+
## 背景
38+
39+
- Point / Point2D 是 SymPy 几何模块的点对象。构造函数会对坐标做一系列入参校验,其中一条契约是:明确为虚数的坐标(如 Point(I, 2))会被 ValueError 拒绝。
40+
- evaluate(False) 会全局关闭自动求值:像 im(1) 这样的函数调用会保持未求值形式,而不是立即化简为 0。SymPy 的性质查询(如 is_zero)采用三值逻辑:True / False / None(未知)。
41+
42+
## 答案与解析
43+
44+
参考答案在 `expected-findings.json``rubric.md` 中(剧透注意)。在线做题时提交 review 后自动展示。
45+
46+
## 题目来源
4547

46-
- 是否能正确判断这个 AI PR 是否可以合并。
47-
- 是否识别核心风险:The patch disables the imaginary-coordinate safety check whenever evaluate is false.
48-
- 是否说明原有行为边界和 AI patch 改变了什么。
49-
- 是否指出新增测试覆盖不足。
50-
- 是否给出可执行的修复建议,而不是只说“加测试”。
48+
本题改编自真实工程问题(你审核的 `ai-pr.diff` 是 AgentCode 改编的训练补丁,不是上游最终修复)。上游链接在 `metadata.json``source` 字段中,建议做完题再看。
Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,43 @@
11
diff --git a/sympy/geometry/point.py b/sympy/geometry/point.py
2-
index 73c20334d09e..ai0000000000 100644
2+
index 73c20334d..5c8636ee6 100644
33
--- a/sympy/geometry/point.py
44
+++ b/sympy/geometry/point.py
5-
@@ -149,7 +149,7 @@ def __new__(cls, *args, **kwargs):
5+
@@ -149,9 +149,12 @@ def __new__(cls, *args, **kwargs):
66
if any(coords[dim:]):
77
raise ValueError('Nonzero coordinates cannot be removed.')
88
- if any(a.is_number and im(a) for a in coords):
9+
+ # With automatic evaluation disabled, im(a) stays unevaluated (e.g.
10+
+ # im(1)), which made perfectly real coordinates look imaginary
11+
+ # (issue #22684). Only run this validation when evaluation is on.
912
+ if evaluate and any(a.is_number and im(a) for a in coords):
1013
raise ValueError('Imaginary coordinates are not permitted.')
1114
if not all(isinstance(a, Expr) for a in coords):
1215
raise TypeError('Coordinates must be valid SymPy expressions.')
16+
17+
# pad with zeros appropriately
18+
coords = coords[:dim] + (S.Zero,)*(dim - len(coords))
1319
diff --git a/sympy/geometry/tests/test_point.py b/sympy/geometry/tests/test_point.py
20+
index 930d7fec2..8a1c2b3d4 100644
21+
--- a/sympy/geometry/tests/test_point.py
22+
+++ b/sympy/geometry/tests/test_point.py
1423
@@ -1,5 +1,6 @@
1524
from sympy.core.basic import Basic
16-
from sympy.core.numbers import (I, Rational, pi)
25+
from sympy.core.numbers import (I, Float, Rational, pi)
1726
+from sympy.core.parameters import evaluate
18-
@@ -452,6 +453,12 @@ def test__normalize_dimension():
27+
from sympy.core.singleton import S
28+
from sympy.core.symbol import Symbol
29+
from sympy.core.sympify import sympify
30+
@@ -452,6 +453,13 @@ def test__normalize_dimension():
31+
assert Point._normalize_dimension(Point(1, 2), Point(3, 4, 0)) == [
32+
Point(1, 2, 0), Point(3, 4, 0)]
33+
34+
+
1935
+def test_issue_22684():
20-
+ # Used to give an error
36+
+ # Used to raise 'Imaginary coordinates are not permitted.'
2137
+ with evaluate(False):
22-
+ Point(1, 2)
38+
+ p = Point(1, 2)
39+
+ assert p == Point(1, 2)
40+
+
41+
42+
def test_direction_cosine():
43+
p1 = Point3D(0, 0, 0)

challenges/review/001-sympy-point2d-ai-patch/expected-findings.json

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,53 @@
11
{
22
"canMerge": false,
3+
"mergeRationale": "Gating the imaginary-coordinate check on evaluate disables a documented validation: Point(I, 2) is silently accepted under evaluate(False).",
34
"requiredFindings": [
45
{
56
"id": "imaginary-coordinate-regression",
67
"severity": "high",
78
"summary": "The patch disables the imaginary-coordinate safety check whenever evaluate is false.",
89
"expectedReasoning": "Point(I, 2) inside evaluate(False) can bypass the ValueError, so the original coordinate contract is weakened.",
9-
"acceptableFix": "Keep the guard active and reject only coordinates whose imaginary part is definitely non-zero, e.g. im(a).is_zero is False."
10+
"acceptableFix": "Keep the guard active and reject only coordinates whose imaginary part is definitely non-zero, e.g. im(a).is_zero is False.",
11+
"matchTerms": [
12+
["point(i"],
13+
["虚数", "evaluate"],
14+
["虚数", "跳过"],
15+
["虚数", "绕过"],
16+
["虚数", "不再"],
17+
["imaginary", "skip"],
18+
["imaginary", "bypass"],
19+
["imaginary", "disable"]
20+
]
1021
},
1122
{
1223
"id": "missing-negative-test",
1324
"severity": "medium",
1425
"summary": "The new test covers only the happy path.",
1526
"expectedReasoning": "It proves Point(1, 2) under evaluate(False), but not that invalid imaginary coordinates are still rejected.",
16-
"acceptableFix": "Add a negative test asserting Point(I, 2) still raises under evaluate(False)."
27+
"acceptableFix": "Add a negative test asserting Point(I, 2) still raises under evaluate(False).",
28+
"matchTerms": [
29+
["测试", "虚数"],
30+
["测试", "point(i"],
31+
["test", "imaginary"],
32+
["test", "point(i"],
33+
["负向", "测试"],
34+
["negative", "test"],
35+
["happy path"]
36+
]
1737
}
1838
],
1939
"optionalFindings": [
2040
{
2141
"id": "narrower-predicate",
2242
"severity": "medium",
23-
"summary": "The predicate should encode certainty about the imaginary part, not the global evaluation mode."
43+
"summary": "The predicate should encode certainty about the imaginary part, not the global evaluation mode.",
44+
"matchTerms": [
45+
["is_zero"],
46+
["确定", "非零"],
47+
["definitely"],
48+
["三值"],
49+
["unknown", "reject"]
50+
]
2451
}
2552
],
2653
"disallowedConclusions": [

0 commit comments

Comments
 (0)