Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 95 additions & 0 deletions docs/guides/same-cohort-average-check.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
# 检查方法:聚合时分子与分母必须来自同一队列

> 适用范围:任何"求和 / 求平均 / 算比例"的代码,无论在 Python 里还是 SQL 里。
> 来源:已合并的 PR [#58](https://github.com/XiaoCow666/CodeSense/pull/58)。

## 1. 历史记录复盘

**问题位置**:`utils/maturity_calculator.py` 的 `calculate_maturity_components`,
成熟度分量 phi_grad(φ_grad,进步梯度)。

**触发**:φ_grad 把一个学生的提交按时间切成前后两半,比较两半平均分。
`Submission.score` 是可空列,"已提交但尚未评分"的提交 `score=None` 是真实
数据状态,不是异常输入。

**旧代码的问题**(修复前):

```python
# 分子:只累加 truthy 分数(None 被过滤掉)
growth_sum = sum(s.score for s in half if s.score)
# 分母:用整半长度(None 仍被计数)
avg = growth_sum / len(half)
```

分子排除了 None,分母却包含 None——口径不一致等价于把未评分提交当 0 分。
两半缺失率不同时,梯度方向都会反,例如:

| 提交分数(按时间) | 真实语义 | 修复前 φ_grad |
|---|---|---|
| `[40, 40, None, 43]` | 40 → 43,进步 | **0**(报成大幅退步) |
| `[None, 43, 40, 40]` | 43 → 40,小幅退步 | **100**(报成大幅进步) |

**处理**:两半分别只收集 `score is not None` 的提交,分子分母基于同一批
已评分提交;任一半没有可评分提交时没有可比较的均值,保持中性默认 50。

**验证**:先写失败测试(RED:3 failed / 5 passed),修复后同一命令
GREEN:8 passed;全量回归无失败。

**同源变体**(已合并的 PR [#67](https://github.com/XiaoCow666/CodeSense/pull/67)):
`last_activity_at is None` 把"调用者省略参数"和"批量查询未命中、显式传入
None"两种语义合进同一分支,导致批量列表 N+1。教训与本方法一致:**一个分支
里不要处理两种口径**,用独立 sentinel 区分。

## 2. 五步检查步骤

下次写或评审聚合代码时按此步骤走一遍:

1. **找聚合**:定位 `sum / avg / mean / 比例 / 百分率` 等计算,包括 SQL 聚合函数。
2. **写两个集合**:明确列出分子实际累加的是哪些记录、分母实际计数的是哪些记录。
3. **标口径差异**:逐条检查两个集合的过滤条件是否一致——`None` 处理、
`if x` 这类 truthy 过滤、SQL `WHERE` 条件、空集合默认值。
4. **统一到同一队列**:让分子分母来自同一批记录;缺失时显式给中性结果或
提前返回,不要让缺失项静默进入分母。
5. **RED → GREEN 验证**:构造一个"缺失项分布不均"的输入(缺失只出现在
分子侧或只出现在一侧分组),确认修复前断言失败、修复后通过;再补一个
全部有值的正常路径用例,确认正常路径不被改坏。

## 3. 用新例子验证方法:首页平均分查询

**位置**:`routes/main.py` 的 `home` 视图。

```python
average_score_query = db.session.query(func.avg(Submission.score)).filter(
Submission.student_id == student_id,
Submission.assignment_id.in_(all_assigned_ids)
).scalar()
```

**按五步检查**:

1. 聚合:SQL `AVG(score)`。
2. 分子集合:匹配 `WHERE` 条件的行的 `score`;分母集合:同一批匹配行——
SQL 聚合函数没有独立的显式分母。
3. 口径差异:无。`AVG` 在数据库内部自动**忽略** NULL 行,分子分母跳过的
是同一批记录,口径天然一致。
4. 是否需要统一:不需要。
5. 验证方式:关注点随之转移到下游
`average_score = average_score_query if average_score_query else 0`——
它处理的是"没有任何已评分提交"时 `AVG` 返回 NULL 的情况,回退为 0,
语义正确。

**判定:安全,无需修改。**

作为对照,若有人把这段改成 Python 侧手工聚合,就会重新落入 #58 的缺陷:

```python
# 危险写法:分子过滤 None,分母 len() 含 None,口径不一致
scores = [s.score for s in rows]
average = sum(v for v in scores if v is not None) / len(scores)
```

## 4. 何时使用这条方法

- 评审包含均值、比率、完成率、得分汇总的改动时;
- 改动过滤条件(尤其新增/放宽 `None`、空值处理)后;
- 数据模型中字段可空,而聚合逻辑假设字段总有值时。
43 changes: 43 additions & 0 deletions tests/test_same_cohort_check_doc.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
"""方法文档的源码级测试。

仅用标准库读取指南 Markdown 文本:不 import Flask、不连数据库/Redis、
不启动应用。验证三件事:引用了已合并的历史 PR、列出可操作的检查步骤、
并用一个新例子给出明确判定。
"""
from pathlib import Path

GUIDE = (
Path(__file__).resolve().parent.parent
/ "docs"
/ "guides"
/ "same-cohort-average-check.md"
)


def _read():
return GUIDE.read_text(encoding="utf-8")


def test_guide_exists_and_references_merged_pr58():
text = _read()
assert "https://github.com/XiaoCow666/CodeSense/pull/58" in text
# 历史问题的具体落点,防止引用写成空话
assert "phi_grad" in text
assert "maturity_calculator" in text


def test_guide_lists_actionable_check_steps():
text = _read()
for keyword in ("步骤", "分子", "分母"):
assert keyword in text
# 方法核心:分子分母必须来自同一批记录
assert "同一" in text


def test_guide_validates_method_with_new_func_avg_example():
text = _read()
# 新例子:学生首页平均分查询
assert "func.avg" in text
# 必须给出可核验的明确判定,而不是只把例子摆出来
assert "安全" in text
assert "忽略" in text # SQL AVG 忽略 NULL 是判定理由
Loading