Re-structure the project.
This commit is contained in:
405
docs/CODE_REVIEW_REPORT.md
Normal file
405
docs/CODE_REVIEW_REPORT.md
Normal file
@@ -0,0 +1,405 @@
|
||||
# Invoice Master POC v2 - 代码审查报告
|
||||
|
||||
**审查日期**: 2026-01-22
|
||||
**代码库规模**: 67 个 Python 源文件,约 22,434 行代码
|
||||
**测试覆盖率**: ~40-50%
|
||||
|
||||
---
|
||||
|
||||
## 执行摘要
|
||||
|
||||
### 总体评估:**良好(B+)**
|
||||
|
||||
**优势**:
|
||||
- ✅ 清晰的模块化架构,职责分离良好
|
||||
- ✅ 使用了合适的数据类和类型提示
|
||||
- ✅ 针对瑞典发票的全面规范化逻辑
|
||||
- ✅ 空间索引优化(O(1) token 查找)
|
||||
- ✅ 完善的降级机制(YOLO 失败时的 OCR fallback)
|
||||
- ✅ 设计良好的 Web API 和 UI
|
||||
|
||||
**主要问题**:
|
||||
- ❌ 支付行解析代码重复(3+ 处)
|
||||
- ❌ 长函数(`_normalize_customer_number` 127 行)
|
||||
- ❌ 配置安全问题(明文数据库密码)
|
||||
- ❌ 异常处理不一致(到处都是通用 Exception)
|
||||
- ❌ 缺少集成测试
|
||||
- ❌ 魔法数字散布各处(0.5, 0.95, 300 等)
|
||||
|
||||
---
|
||||
|
||||
## 1. 架构分析
|
||||
|
||||
### 1.1 模块结构
|
||||
|
||||
```
|
||||
src/
|
||||
├── inference/ # 推理管道核心
|
||||
│ ├── pipeline.py (517 行) ⚠️
|
||||
│ ├── field_extractor.py (1,347 行) 🔴 太长
|
||||
│ └── yolo_detector.py
|
||||
├── web/ # FastAPI Web 服务
|
||||
│ ├── app.py (765 行) ⚠️ HTML 内联
|
||||
│ ├── routes.py (184 行)
|
||||
│ └── services.py (286 行)
|
||||
├── ocr/ # OCR 提取
|
||||
│ ├── paddle_ocr.py
|
||||
│ └── machine_code_parser.py (919 行) 🔴 太长
|
||||
├── matcher/ # 字段匹配
|
||||
│ └── field_matcher.py (875 行) ⚠️
|
||||
├── utils/ # 共享工具
|
||||
│ ├── validators.py
|
||||
│ ├── text_cleaner.py
|
||||
│ ├── fuzzy_matcher.py
|
||||
│ ├── ocr_corrections.py
|
||||
│ └── format_variants.py (610 行)
|
||||
├── processing/ # 批处理
|
||||
├── data/ # 数据管理
|
||||
└── cli/ # 命令行工具
|
||||
```
|
||||
|
||||
### 1.2 推理流程
|
||||
|
||||
```
|
||||
PDF/Image 输入
|
||||
↓
|
||||
渲染为图片 (pdf/renderer.py)
|
||||
↓
|
||||
YOLO 检测 (yolo_detector.py) - 检测字段区域
|
||||
↓
|
||||
字段提取 (field_extractor.py)
|
||||
├→ OCR 文本提取 (ocr/paddle_ocr.py)
|
||||
├→ 规范化 & 验证
|
||||
└→ 置信度计算
|
||||
↓
|
||||
交叉验证 (pipeline.py)
|
||||
├→ 解析 payment_line 格式
|
||||
├→ 从 payment_line 提取 OCR/Amount/Account
|
||||
└→ 与检测字段验证,payment_line 值优先
|
||||
↓
|
||||
降级 OCR(如果关键字段缺失)
|
||||
├→ 全页 OCR
|
||||
└→ 正则提取
|
||||
↓
|
||||
InferenceResult 输出
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 2. 代码质量问题
|
||||
|
||||
### 2.1 长函数(>50 行)🔴
|
||||
|
||||
| 函数 | 文件 | 行数 | 复杂度 | 问题 |
|
||||
|------|------|------|--------|------|
|
||||
| `_normalize_customer_number()` | field_extractor.py | **127** | 极高 | 4 层模式匹配,7+ 正则,复杂评分 |
|
||||
| `_cross_validate_payment_line()` | pipeline.py | **127** | 极高 | 核心验证逻辑,8+ 条件分支 |
|
||||
| `_normalize_bankgiro()` | field_extractor.py | 62 | 高 | Luhn 验证 + 多种降级 |
|
||||
| `_normalize_plusgiro()` | field_extractor.py | 63 | 高 | 类似 bankgiro |
|
||||
| `_normalize_payment_line()` | field_extractor.py | 74 | 高 | 4 种正则模式 |
|
||||
| `_normalize_amount()` | field_extractor.py | 78 | 高 | 多策略降级 |
|
||||
|
||||
**示例问题** - `_normalize_customer_number()` (第 776-902 行):
|
||||
```python
|
||||
def _normalize_customer_number(self, text: str):
|
||||
# 127 行函数,包含:
|
||||
# - 4 个嵌套的 if/for 循环
|
||||
# - 7 种不同的正则模式
|
||||
# - 5 个评分机制
|
||||
# - 处理有标签和无标签格式
|
||||
```
|
||||
|
||||
**建议**: 拆分为:
|
||||
- `_find_customer_code_patterns()`
|
||||
- `_find_labeled_customer_code()`
|
||||
- `_score_customer_candidates()`
|
||||
|
||||
### 2.2 代码重复 🔴
|
||||
|
||||
**支付行解析(3+ 处重复实现)**:
|
||||
|
||||
1. `_parse_machine_readable_payment_line()` (pipeline.py:217-252)
|
||||
2. `MachineCodeParser.parse()` (machine_code_parser.py:919 行)
|
||||
3. `_normalize_payment_line()` (field_extractor.py:632-705)
|
||||
|
||||
所有三处都实现类似的正则模式:
|
||||
```
|
||||
格式: # <OCR> # <Kronor> <Öre> <Type> > <Account>#<Check>#
|
||||
```
|
||||
|
||||
**Bankgiro/Plusgiro 验证(重复)**:
|
||||
- `validators.py`: `is_valid_bankgiro()`, `format_bankgiro()`
|
||||
- `field_extractor.py`: `_normalize_bankgiro()`, `_normalize_plusgiro()`, `_luhn_checksum()`
|
||||
- `normalizer.py`: `normalize_bankgiro()`, `normalize_plusgiro()`
|
||||
- `field_matcher.py`: 类似匹配逻辑
|
||||
|
||||
**建议**: 创建统一模块:
|
||||
```python
|
||||
# src/common/payment_line_parser.py
|
||||
class PaymentLineParser:
|
||||
def parse(text: str) -> PaymentLineResult
|
||||
|
||||
# src/common/giro_validator.py
|
||||
class GiroValidator:
|
||||
def validate_and_format(value: str, giro_type: str) -> str
|
||||
```
|
||||
|
||||
### 2.3 错误处理不一致 ⚠️
|
||||
|
||||
**通用异常捕获(31 处)**:
|
||||
```python
|
||||
except Exception as e: # 代码库中 31 处
|
||||
result.errors.append(str(e))
|
||||
```
|
||||
|
||||
**问题**:
|
||||
- 没有捕获特定错误类型
|
||||
- 通用错误消息丢失上下文
|
||||
- 第 142-147 行 (routes.py): 捕获所有异常,返回 500 状态
|
||||
|
||||
**当前写法** (routes.py:142-147):
|
||||
```python
|
||||
try:
|
||||
service_result = inference_service.process_pdf(...)
|
||||
except Exception as e: # 太宽泛
|
||||
logger.error(f"Error processing document: {e}")
|
||||
raise HTTPException(status_code=500, detail=str(e))
|
||||
```
|
||||
|
||||
**改进建议**:
|
||||
```python
|
||||
except FileNotFoundError:
|
||||
raise HTTPException(status_code=400, detail="PDF 文件未找到")
|
||||
except PyMuPDFError:
|
||||
raise HTTPException(status_code=400, detail="无效的 PDF 格式")
|
||||
except OCRError:
|
||||
raise HTTPException(status_code=503, detail="OCR 服务不可用")
|
||||
```
|
||||
|
||||
### 2.4 配置安全问题 🔴
|
||||
|
||||
**config.py 第 24-30 行** - 明文凭据:
|
||||
```python
|
||||
DATABASE = {
|
||||
'host': '192.168.68.31', # 硬编码 IP
|
||||
'user': 'docmaster', # 硬编码用户名
|
||||
'password': 'nY6LYK5d', # 🔴 明文密码!
|
||||
'database': 'invoice_master'
|
||||
}
|
||||
```
|
||||
|
||||
**建议**:
|
||||
```python
|
||||
DATABASE = {
|
||||
'host': os.getenv('DB_HOST', 'localhost'),
|
||||
'user': os.getenv('DB_USER', 'docmaster'),
|
||||
'password': os.getenv('DB_PASSWORD'), # 从环境变量读取
|
||||
'database': os.getenv('DB_NAME', 'invoice_master')
|
||||
}
|
||||
```
|
||||
|
||||
### 2.5 魔法数字 ⚠️
|
||||
|
||||
| 值 | 位置 | 用途 | 问题 |
|
||||
|---|------|------|------|
|
||||
| 0.5 | 多处 | 置信度阈值 | 不可按字段配置 |
|
||||
| 0.95 | pipeline.py | payment_line 置信度 | 无说明 |
|
||||
| 300 | 多处 | DPI | 硬编码 |
|
||||
| 0.1 | field_extractor.py | BBox 填充 | 应为配置 |
|
||||
| 72 | 多处 | PDF 基础 DPI | 公式中的魔法数字 |
|
||||
| 50 | field_extractor.py | 客户编号评分加分 | 无说明 |
|
||||
|
||||
**建议**: 提取到配置:
|
||||
```python
|
||||
INFERENCE_CONFIG = {
|
||||
'confidence_threshold': 0.5,
|
||||
'payment_line_confidence': 0.95,
|
||||
'dpi': 300,
|
||||
'bbox_padding': 0.1,
|
||||
}
|
||||
```
|
||||
|
||||
### 2.6 命名不一致 ⚠️
|
||||
|
||||
**字段名称不一致**:
|
||||
- YOLO 类名: `invoice_number`, `ocr_number`, `supplier_org_number`
|
||||
- 字段名: `InvoiceNumber`, `OCR`, `supplier_org_number`
|
||||
- CSV 列名: 可能又不同
|
||||
- 数据库字段名: 另一种变体
|
||||
|
||||
映射维护在多处:
|
||||
- `yolo_detector.py` (90-100 行): `CLASS_TO_FIELD`
|
||||
- 多个其他位置
|
||||
|
||||
---
|
||||
|
||||
## 3. 测试分析
|
||||
|
||||
### 3.1 测试覆盖率
|
||||
|
||||
**测试文件**: 13 个
|
||||
- ✅ 覆盖良好: field_matcher, normalizer, payment_line_parser
|
||||
- ⚠️ 中等覆盖: field_extractor, pipeline
|
||||
- ❌ 覆盖不足: web 层, CLI, 批处理
|
||||
|
||||
**估算覆盖率**: 40-50%
|
||||
|
||||
### 3.2 缺失的测试用例 🔴
|
||||
|
||||
**关键缺失**:
|
||||
1. 交叉验证逻辑 - 最复杂部分,测试很少
|
||||
2. payment_line 解析变体 - 多种实现,边界情况不清楚
|
||||
3. OCR 错误纠正 - 不同策略的复杂逻辑
|
||||
4. Web API 端点 - 没有请求/响应测试
|
||||
5. 批处理 - 多 worker 协调未测试
|
||||
6. 降级 OCR 机制 - YOLO 检测失败时
|
||||
|
||||
---
|
||||
|
||||
## 4. 架构风险
|
||||
|
||||
### 🔴 关键风险
|
||||
|
||||
1. **配置安全** - config.py 中明文数据库凭据(24-30 行)
|
||||
2. **错误恢复** - 宽泛的异常处理掩盖真实问题
|
||||
3. **可测试性** - 硬编码依赖阻止单元测试
|
||||
|
||||
### 🟡 高风险
|
||||
|
||||
1. **代码可维护性** - 支付行解析重复
|
||||
2. **可扩展性** - 没有长时间推理的异步处理
|
||||
3. **扩展性** - 添加新字段类型会很困难
|
||||
|
||||
### 🟢 中等风险
|
||||
|
||||
1. **性能** - 懒加载有帮助,但 ORM 查询未优化
|
||||
2. **文档** - 大部分足够但可以更好
|
||||
|
||||
---
|
||||
|
||||
## 5. 优先级矩阵
|
||||
|
||||
| 优先级 | 行动 | 工作量 | 影响 |
|
||||
|--------|------|--------|------|
|
||||
| 🔴 关键 | 修复配置安全(环境变量) | 1 小时 | 高 |
|
||||
| 🔴 关键 | 添加集成测试 | 2-3 天 | 高 |
|
||||
| 🔴 关键 | 文档化错误处理策略 | 4 小时 | 中 |
|
||||
| 🟡 高 | 统一 payment_line 解析 | 1-2 天 | 高 |
|
||||
| 🟡 高 | 提取规范化到子模块 | 2-3 天 | 中 |
|
||||
| 🟡 高 | 添加依赖注入 | 2-3 天 | 中 |
|
||||
| 🟡 高 | 拆分长函数 | 2-3 天 | 低 |
|
||||
| 🟢 中 | 提高测试覆盖率到 70%+ | 3-5 天 | 高 |
|
||||
| 🟢 中 | 提取魔法数字 | 4 小时 | 低 |
|
||||
| 🟢 中 | 标准化命名约定 | 1-2 天 | 中 |
|
||||
|
||||
---
|
||||
|
||||
## 6. 具体文件建议
|
||||
|
||||
### 高优先级(代码质量)
|
||||
|
||||
| 文件 | 问题 | 建议 |
|
||||
|------|------|------|
|
||||
| `field_extractor.py` | 1,347 行;6 个长规范化方法 | 拆分为 `normalizers/` 子模块 |
|
||||
| `pipeline.py` | 127 行 `_cross_validate_payment_line()` | 提取到单独的 `CrossValidator` 类 |
|
||||
| `field_matcher.py` | 875 行;复杂匹配逻辑 | 拆分为 `matching/` 子模块 |
|
||||
| `config.py` | 硬编码凭据(第 29 行) | 使用环境变量 |
|
||||
| `machine_code_parser.py` | 919 行;payment_line 解析 | 与 pipeline 解析合并 |
|
||||
|
||||
### 中优先级(重构)
|
||||
|
||||
| 文件 | 问题 | 建议 |
|
||||
|------|------|------|
|
||||
| `app.py` | 765 行;HTML 内联在 Python 中 | 提取到 `templates/` 目录 |
|
||||
| `autolabel.py` | 753 行;批处理逻辑 | 提取 worker 函数到模块 |
|
||||
| `format_variants.py` | 610 行;变体生成 | 考虑策略模式 |
|
||||
|
||||
---
|
||||
|
||||
## 7. 建议行动
|
||||
|
||||
### 第 1 阶段:关键修复(1 周)
|
||||
|
||||
1. **配置安全** (1 小时)
|
||||
- 移除 config.py 中的明文密码
|
||||
- 添加环境变量支持
|
||||
- 更新 README 说明配置
|
||||
|
||||
2. **错误处理标准化** (1 天)
|
||||
- 定义自定义异常类
|
||||
- 替换通用 Exception 捕获
|
||||
- 添加错误代码常量
|
||||
|
||||
3. **添加关键集成测试** (2 天)
|
||||
- 端到端推理测试
|
||||
- payment_line 交叉验证测试
|
||||
- API 端点测试
|
||||
|
||||
### 第 2 阶段:重构(2-3 周)
|
||||
|
||||
4. **统一 payment_line 解析** (2 天)
|
||||
- 创建 `src/common/payment_line_parser.py`
|
||||
- 合并 3 处重复实现
|
||||
- 迁移所有调用方
|
||||
|
||||
5. **拆分 field_extractor.py** (3 天)
|
||||
- 创建 `src/inference/normalizers/` 子模块
|
||||
- 每个字段类型一个文件
|
||||
- 提取共享验证逻辑
|
||||
|
||||
6. **拆分长函数** (2 天)
|
||||
- `_normalize_customer_number()` → 3 个函数
|
||||
- `_cross_validate_payment_line()` → CrossValidator 类
|
||||
|
||||
### 第 3 阶段:改进(1-2 周)
|
||||
|
||||
7. **提高测试覆盖率** (5 天)
|
||||
- 目标:70%+ 覆盖率
|
||||
- 专注于验证逻辑
|
||||
- 添加边界情况测试
|
||||
|
||||
8. **配置管理改进** (1 天)
|
||||
- 提取所有魔法数字
|
||||
- 创建配置文件(YAML)
|
||||
- 添加配置验证
|
||||
|
||||
9. **文档改进** (2 天)
|
||||
- 添加架构图
|
||||
- 文档化所有私有方法
|
||||
- 创建贡献指南
|
||||
|
||||
---
|
||||
|
||||
## 附录 A:度量指标
|
||||
|
||||
### 代码复杂度
|
||||
|
||||
| 类别 | 计数 | 平均行数 |
|
||||
|------|------|----------|
|
||||
| 源文件 | 67 | 334 |
|
||||
| 长文件 (>500 行) | 12 | 875 |
|
||||
| 长函数 (>50 行) | 23 | 89 |
|
||||
| 测试文件 | 13 | 298 |
|
||||
|
||||
### 依赖关系
|
||||
|
||||
| 类型 | 计数 |
|
||||
|------|------|
|
||||
| 外部依赖 | ~25 |
|
||||
| 内部模块 | 10 |
|
||||
| 循环依赖 | 0 ✅ |
|
||||
|
||||
### 代码风格
|
||||
|
||||
| 指标 | 覆盖率 |
|
||||
|------|--------|
|
||||
| 类型提示 | 80% |
|
||||
| Docstrings (公开) | 80% |
|
||||
| Docstrings (私有) | 40% |
|
||||
| 测试覆盖率 | 45% |
|
||||
|
||||
---
|
||||
|
||||
**生成日期**: 2026-01-22
|
||||
**审查者**: Claude Code
|
||||
**版本**: v2.0
|
||||
Reference in New Issue
Block a user