Files
dataClean/CODE_REVIEW.md
T
2026-06-12 16:04:03 +08:00

460 lines
16 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# dataClean 代码审核报告
> 审核日期:2026-06-12
> 审核范围:后端(FastAPI + SQLAlchemy + APScheduler / 前端(Vue 3 + Element Plus / 配置与部署
> 审核人:opencode
## 项目概览
- **技术栈**FastAPI 0.115 + SQLAlchemy 2.0 + SQLite + APScheduler 3.10(后端) / Vue 3.4 + Element Plus 2.6 + Vite 5(前端) / OpenAI 兼容 LLM
- **代码规模**:约 1.5k 行 Python + 1.2k 行 Vue
- **目标**:从 rssKeeper 拉取文章,做摘要/分类/打分/去重/简报生成,提供 Web UI
- **整体评价**:模块化清晰、`README.md` 完整可读,但存在安全、性能与正确性方面的隐患。
---
## 审核结论一览
| 严重等级 | 数量 | 含义 |
|----------|------|------|
| 🔴 严重 | 7 | 影响线上数据安全与正确性,上线前必须修复 |
| 🟡 中等 | 13 | 影响可维护性、时序正确性、可观测性,建议近期修复 |
| 🟢 轻量 | 10 | 代码风格、健壮性细节,可持续改进 |
---
## 🔴 严重问题(上线前必须修复)
### 1. CORS 配置错误且过于宽松
**文件**`main.py:72-78`
```python
app.add_middleware(
CORSMiddleware,
allow_origins=["*"],
allow_credentials=True,
allow_methods=["*"],
allow_headers=["*"],
)
```
- `allow_origins=["*"]``allow_credentials=True` 同时启用被 Starlette 视为非法组合。
- 后端无任何鉴权(见 #2),任何网站都能通过浏览器代表"已登录用户"调用 API。
**建议**:生产环境收敛到具体域名,关闭 credentials,或删除 CORSWeb UI 走同源代理)。
---
### 2. 后端 API 无任何鉴权
所有接口(`/api/settings``/api/tasks/summarize``/api/taxonomy/bootstrap?force=true`)公开可访问:
- `Settings.vue:24-35` 可在 Web UI 直接改写 LLM API Key。
- `Tasks.vue:18-26` 可未经授权立即触发高额 LLM 调用。
- 两者叠加,**任何能访问 7331 端口的访客都能改 key、消耗 token**。
**建议**:反代层加 BasicAuth,或在 `main.py``Depends(verify_token)`
---
### 3. 去重任务破坏历史数据
**文件**`app/deduplicator.py:146-152`
```python
old_groups = db.query(DuplicateGroup).all() # 拉取全部
for og in old_groups:
for art in og.articles:
art.duplicate_group_id = None
art.is_representative = False
db.delete(og)
db.commit()
```
去重仅按"当天"过滤文章(line 158-165),但**清空阶段删除的是所有日期的 `DuplicateGroup`**,且把历史上所有文章的 `is_representative` 重置为 `False`
- 后果:每日 8:00 简报生成后,**所有历史文章的重复组信息都被清空**。
- `brief.py:99-106` 依靠 `is_representative=True OR duplicate_group_id IS NULL` 取代表文章,缺一会导致简报里出现全部 N 篇文章。
**建议**:只删除 `representative_article_id` 属于当天文章的去重组,或在 `DuplicateGroup` 上加 `brief_date` 字段。
---
### 4. `_with_db` 装饰器静默吞掉所有异常
**文件**`scheduler.py:40-51`
```python
except Exception as exc:
logger.error("定时任务 %s 执行失败: %s", func.__name__, exc)
```
任务失败仅有日志,**没有**
- 任务状态持久化(前端无法知道哪些任务最近失败过)。
- 告警 / 通知。
- 失败指标(Prometheus 等)。
如果 LLM 配额耗尽或 rssKeeper 挂掉,**服务会假装正常跑了 N 天**。
**建议**:建 `JobRunLog` 表记录 `(job_id, start, end, status, error)`,或在 Web UI 暴露上次运行结果。
---
### 5. 手动任务与定时任务可并发执行
**文件**`main.py:248-267``scheduler.py:104-133`
`max_instances=1` 仅对 APScheduler 注册的实例生效,不约束 `POST /api/tasks/summarize`。一旦同时执行,`fetch_and_summarize` 内部有重复 `commit()`,可能引发 unique 约束冲突或写脏数据。
**建议**:在 `main.py` 用全局 `threading.Lock` 包裹任务函数。
---
### 6. 去重算法 O(n²) 性能
**文件**`app/deduplicator.py:88-113`
`n` 篇文章做 BFS 嵌套循环,每对调用 `SequenceMatcher`(也是 O(L²))。200 篇时是 4 万次 `SequenceMatcher` + TF-IDF 矩阵计算,**单日任务常常跑 5–10 分钟**。
**建议**
- 标题长度 hash → 桶聚类后再做 pair 比较(minhash / LSH 更佳)。
- 内容相似度先按 TF-IDF 矩阵做阈值筛选 top-K,再做精确比较。
---
### 7. Dockerfile 以 root 运行且未指定 USER
**文件**`Dockerfile:10-26`
`FROM python:3.12-slim` 后未建非 root 用户,gunicorn/uvicorn 全部以 root 跑。一旦 Web 漏洞被利用,攻击者直接拿到容器 root。
**建议**
```dockerfile
RUN useradd --create-home --uid 1000 app
USER app
```
---
## 🟡 中等问题(影响正确性 / 可维护性)
### 8. 时区处理混乱
- `scheduler.py:35``timezone="Asia/Shanghai"`
- `scorer.py:49``brief.py:73` 等都用 `datetime.utcnow()`
- `summarizer.py:86` 把 ISO 时间解析为带 tzinfo,但 `scorer.py:55-58``replace(tzinfo=None)` 强行丢掉。
`score_articles` 内部用 UTC 当前时间,`_freshness_score` 在 24 小时分界点附近会因 tzinfo 一致性问题差几个小时。
**建议**:统一用 `datetime.now(timezone.utc)` 持久化,明确表里存的时区。
---
### 9. `datetime.utcnow()` 已被弃用
Python 3.12+ 标注 `datetime.utcnow()` 为 deprecated。
涉及文件:
- `models.py:25,45`
- `summarizer.py:137`
- `scorer.py:49`
- `brief.py:73,154`
- `settings_manager.py:98`
**建议**:替换为 `datetime.now(timezone.utc)`
---
### 10. 重复性分数公式与文档不符
**文件**`app/scorer.py:83-91` + `deduplicator.py:194`
```python
member_ids = [unique_articles[i].id for i in cluster] # 包含代表,最少 2
...
dup_count = max(len(group.member_article_ids), 1) # >= 2
compute_duplication_score(2) -> 25.0 # 不是 0
```
注释说 "1 次为 0 分",实际最小是 2,永远不会得 0。
**建议**:用 `len(member_article_ids) - 1`(非代表成员数),或调整公式。
---
### 11. 标签筛选性能差且语义不严谨
**文件**`main.py:179-180`
```python
if tag:
query = query.filter(EnrichedArticle.tags.contains([tag]))
```
SQLAlchemy 会把整个 JSON 列 `json.dumps` 后做字符串包含比较,**无法走索引**。表大时会全表扫描,且若文章有 `["人工智能"]`,匹配 "人工" 也会命中。
**建议**:建关联表 `article_tags(article_id, tag_name)`,或使用 SQLite JSON 函数 `json_each`
---
### 12. Pydantic v1 风格 Config
**文件**`main.py:99-125`
```python
class Config:
from_attributes = True
```
应改为 Pydantic v2 风格:
```python
model_config = ConfigDict(from_attributes=True)
```
并需 `from pydantic import ConfigDict``ArticleOut.tags: list` 也应改为 `List[str]`,否则对 SQLAlchemy JSON 列不会做反序列化。
---
### 13. `_with_db` 装饰器未保留元信息
**文件**`scheduler.py:40-51`
手写 `wrapper.__name__ = func.__name__`,但缺 `__doc__``__wrapped__`。改用 `@functools.wraps(func)` 更标准。
---
### 14. 前端串行保存 17 个配置项
**文件**`Settings.vue:68-80`
```js
for (const item of settings.value) {
await datacleanApi.updateSetting(item.key, item.value)
}
```
17 个 PUT 串行,任何一个失败就中断且不提示哪些失败。
**建议**:后端加 `PUT /api/settings` 批量接口;前端用 `Promise.allSettled` 或事务式调用。
---
### 15. 分页 total 是 hack
**文件**`Articles.vue:108`
```js
pagination.total = res.length === pagination.size
? pagination.page * pagination.size + 1
: (pagination.page - 1) * pagination.size + res.length
```
`+1` 是为了让 el-pagination 多显示一页按钮的粗暴 hack,**末页判断会出错**(恰好填满时 total 比真实多 1)。
**建议**:后端响应里加 `total` 字段(`/api/articles` 改为 `{items, total}`),前端用真实 total。
---
### 16. 缺数据库迁移
`database.py:34-35``Base.metadata.create_all`
- 加列(如 `EnrichedArticle.is_hidden`)会无报错地忽略。
- 类型变更(`String(128)``String(256)`)会保留旧列。
- 删字段不会清理。
**建议**:引入 Alembic,至少 `alembic init` 起一个 baseline。
---
### 17. `_normalize_title` 字符范围偏窄
**文件**`deduplicator.py:23`
```python
title = re.sub(r"[^\w一-鿿]", " ", title)
```
- `\w` 不含中文,逻辑可接受。
- 鿿是 U+9FFF**U+A000U+FFFF 之间的生僻字 / 部首扩展区 B 字符会被误删**。可用 `[\u4e00-\u9fff]` 或 Python `regex` 库的 `\p{Han}`
---
### 18. Docker 构建镜像源硬编码
**文件**`Dockerfile:5,20`
- `npmmirror.com` 镜像在国内可用,海外构建会慢或超时。
- `tuna.tsinghua.edu.cn` 同上。
**建议**:用 `ARG REGISTRY_MIRROR=...` + `--build-arg` 注入,或在 CI/海外构建时覆盖。
---
### 19. LLM 客户端无 token 计数 / 限流
`ai_client.py` 每次失败抛异常就完事。`fetch_and_summarize``summarizer.py:139-143`)对每篇文章都重试,没有:
- 失败后 cooldown。
- Token 用量统计。
- 限速(OpenAI tier 限流会导致 429)。
**建议**:加 `tenacity` 做指数退避、记录 429 重试、保存 token 消耗日志。
---
### 20. `_get_env_default` 强转字符串丢失类型
**文件**`settings_manager.py:36-39`
```python
return str(value) if value is not None else ""
```
`OPENAI_TIMEOUT=60` 写入数据库变成 `"60"`,再 `apply_db_settings_to_config``int(db_value)` 还原——逻辑 OK,**但**如果用户直接编辑 DB 写入非数字字符串,启动时 `apply_db_settings_to_config` 会捕获失败(`logger.warning` 不会中断),**线上的 `settings.OPENAI_TIMEOUT` 仍是默认值**,行为不可见。
**建议**:失败时启动失败或返回 HTTP 503 明确告知。
---
## 🟢 轻量问题(可优化)
### 21. 前端无错误边界
`App.vue``errorCaptured`,任一视图抛错都白屏。
### 22. 测试覆盖度不足
- `test_deduplicator.py` 测了单簇简单情况,但未覆盖:
- 跨日期去重
- URL 重复但内容不同
- 大簇(>5 篇)
- `deduplicate_articles``old_groups` 清空逻辑(**这是严重 bug**)
- `test_scorer.py` 没测 `_freshness_score`
- 没有 `test_taxonomy.py``test_summarizer.py``test_brief.py``test_settings_manager.py`
- 没有 HTTP 接口测试(`fastapi.testclient`)。
### 23. 日志可观测性
`logging.basicConfig` 文本格式,**没有 request_id、没有结构化字段**。多 worker 时难以追踪。
### 24. `config.py:60` 路径创建副作用
`@property database_path``Settings()` 实例化时 `mkdir`,导入 `config` 就改文件系统。**测试或 CLI 工具 import 该模块就会创建目录**。
**建议**:把目录创建放到 `database.init_db()` 里。
### 25. `feed_category` 字段名耦合假设
**文件**`summarizer.py:96`
假设 rssKeeper 返回字段 `category`,但 README 没写明 rssKeeper 接口契约。应加注释或 Pydantic 模型校验。
### 26. 简报输出目录嵌套过深
**文件**`brief.py:130`
写到 `BRIEF_OUTPUT_DIR/2024-01-01/daily-brief.md`,日期子目录无必要。
### 27. 静态文件兜底逻辑奇怪
**文件**`main.py:330-338`
```python
if not os.path.isdir(static_dir):
frontend_dist = os.path.join(os.path.dirname(__file__), "frontend", "dist")
if os.path.isdir(frontend_dist):
static_dir = frontend_dist
```
- 本地开发用 `npm run dev` 走 Vite 代理,**`frontend/dist` 几乎不存在**,这段代码不工作。
- `app.mount("/", ...)` 会拦截所有未匹配的路由,**包括 `/health``/api/*`**。FastAPI 的注册顺序会把 `app.mount` 放在最末,应该 OK,但建议把静态文件 fallback 用 `html=True` 时显式跳过 `/api``/health`
### 28. README 写"重启后生效"但接口无重启能力
- `main.py:282` 写 "配置已保存,重启服务后生效"。
- 调度间隔是**启动时读取**的(`scheduler.py:97-100`),所以改 `SUMMARIZE_INTERVAL_MINUTES` 真的需要重启。
- 应当提供 `POST /api/restart` 或在 `apply_db_settings_to_config` 之后重新注册 job。
### 29. `models.py:32` `default=list` 是可变默认值陷阱
SQLAlchemy 会克隆 default callable,但**仍建议写成 `default=lambda: list()`** 或在 Python 3.11+ 改用不可变 sentinel。
### 30. 前端无 TypeScript
所有 API 调用都没有类型提示,重构后端响应字段前端不会报错。建议至少加 jsdoc 或逐步迁移到 TS。
---
## 重点修复清单(按 ROI 排序)
| 优先级 | 修复项 | 估计工时 | 风险等级 |
|--------|--------|----------|----------|
| P0 | 加最小化鉴权(BasicAuth 或 token | 1h | 高 |
| P0 | 修复去重 `old_groups` 清空范围 | 30min | 高 |
| P0 | CORS 收敛到生产域名 | 10min | 高 |
| P0 | Dockerfile 加 `USER` | 5min | 高 |
| P1 | 修复分页 total 逻辑(后端 + 前端) | 2h | 中 |
| P1 | 加任务运行日志表 | 3h | 中 |
| P1 | 手动 / 定时任务互斥锁 | 1h | 中 |
| P1 | 修复 `compute_duplication_score` 公式 | 15min | 中 |
| P1 | 前端批量保存配置 | 30min | 中 |
| P2 | 引入 Alembic | 4h | 中 |
| P2 | 去重算法优化(桶聚类 / minhash | 1d | 中 |
| P2 | 统一时区到 UTC | 1h | 低 |
| P2 | LLM 限流 + token 统计 | 4h | 低 |
| P3 | 前端错误边界 + TypeScript | 1d | 低 |
---
## 总评
**项目优点**
- 模块切分清晰(`app/` 下每个职责一个文件)。
- 关键业务逻辑都有单元测试基础。
- 配置双层(env + DB)设计合理。
- 日志、错误信息友好。
- Docker 部署文档完整。
**主要风险**
- **鉴权 + CORS** 双重缺失 → 任何公网访问都是灾难。
- **去重任务数据破坏** → 每日 8:00 简报会持续错误。
- **去重算法性能** → 数据量上来后 O(n²) 不可持续。
**建议路径**
1. **第一步**:修复 P0 安全 / 数据正确性问题(鉴权、CORS、去重 bug、Dockerfile)。
2. **第二步**:补全可观测性(任务运行日志、token 统计、失败告警)。
3. **第三步**:性能优化(去重算法、分页、并发锁、LLM 限流)。
4. **持续改进**:迁移到 TypeScript、引入 Alembic、统一时区、补全测试覆盖。
---
## 附录:文件清单
| 文件 | 行数 | 状态 |
|------|------|------|
| `main.py` | 343 | 需修复(CORS、分页响应、锁、Auth) |
| `config.py` | 63 | 可优化(路径创建副作用) |
| `database.py` | 36 | 建议(Alembic 迁移) |
| `models.py` | 104 | 可优化(JSON 默认值、UTC |
| `scheduler.py` | 151 | 需修复(异常吞掉、时区、互斥) |
| `app/rss_client.py` | 104 | 正常 |
| `app/ai_client.py` | 92 | 建议(限流、重试) |
| `app/taxonomy.py` | 140 | 正常 |
| `app/summarizer.py` | 154 | 可优化(提交边界、重试) |
| `app/tagger.py` | 116 | 正常 |
| `app/scorer.py` | 146 | 需修复(duplication 公式、时区) |
| `app/deduplicator.py` | 216 | 需修复(清空范围、性能) |
| `app/brief.py` | 168 | 可优化(时区、目录嵌套) |
| `app/settings_manager.py` | 185 | 需修复(类型校验失败处理) |
| `tests/conftest.py` | 21 | 正常 |
| `tests/test_deduplicator.py` | 78 | 覆盖不足 |
| `tests/test_scorer.py` | 46 | 覆盖不足 |
| `tests/test_tagger.py` | 43 | 覆盖不足 |
| `Dockerfile` | 27 | 需修复(USER |
| `docker-compose.yml` | 19 | 正常 |
| `frontend/src/api/index.js` | 47 | 正常 |
| `frontend/src/views/*.vue` | - | 需修复(分页、批量保存、错误边界) |