Files
cmhub/docs/phase-2-review.md
T

80 lines
10 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.
# Phase 2 计费核心审核报告(T-201 ~ T-203)
> 审核人:Claude Code(全栈视角)|日期:2026-07-02|结论:**验收通过,质量为三阶段最高,零 P1**。
> Phase 2 是全项目**资金安全最敏感**的部分(`05-coding-rules.md` §8、`04-architecture.md` 第四节)。逐条比对硬约束后:先扣后调/失败必退/并发安全/只追加留痕**全部正确落地**,并有**真线程并发测试在 MySQL 上跑过**。仅剩少量 P2/P3 加固项。
> 本文面向 codex 执行:每条修补项给出「症状 / 位置 / 怎么改 / 怎么验证」。修补任务见 [`06-tasks.md`](06-tasks.md) 的 **T-204**。
## 一、验收核对(全部达标)
| 任务 | 验收要点 | 结果 |
| --- | --- | --- |
| T-201 | User/UserWallet/ApiKey/PointsLedger/CallRecord;`points_balance>=0` 约束;ApiKey 哈希存储;CallRecord 只存 `result_ref`/摘要不 dump raw;admin 注册 | ✅ |
| T-202 | 按「操作 + 能力别名(+ 可选分辨率)」定价;换底层模型不影响计费;缺规则 `no_pricing_rule` | ✅ |
| T-203 | 锁 `UserWallet` 行原子扣减;并发不超扣/不为负;失败退点写流水;含测试 | ✅ |
### §8 资金安全硬约束逐条核对
| §8 约束 | 落地 | 证据 |
| --- | --- | --- |
| 并发安全扣点(事务 + `select_for_update` + DB `>=0`) | ✅ | `services.precharge_call` 事务内锁 wallet 行;`user_wallet` 有 DB 级 `CheckConstraint(points_balance>=0)`(迁移 `0003`) |
| 先扣后调、失败必退 | ✅ | `precharge_call`(扣)/ `mark_call_success` / `refund_call_points`(退)三段与 4.1 时序一一对应 |
| 全程留痕 | ✅ | 每次扣/退写 `PointsLedger`(含 `balance_after`、`ref_call`),建 `CallRecord`;两表 admin **只读**(只追加) |
| 失败退点幂等 | ✅ | `refund_call_points` 先锁 `CallRecord` 行,再查是否已有 REFUND 流水,已退则返回 `refunded=False` 不重复退 |
| 资金改动必须有测试 | ✅ | 并发扣点(真线程)、失败退点幂等、余额不足不写账,均有专测 |
## 二、做对的(勿在修补中回退)
1. **计费时序与 4.1 完全一致**:`precharge_call` = 事务A(锁 wallet→查余额→扣减→建 pending CallRecord + consume Ledger);`refund_call_points` = 事务B(退点 + refund Ledger + 置 failed);`mark_call_success` 只改状态不动余额。三个原语干净可组合,留给 T-302 编排。
2. **真并发测试(非模拟)**:`ConcurrentDebitTests` 用 `TransactionTestCase` + `threading.Barrier` 强制两线程同时扣同一钱包(余额 30 / 各扣 30),断言**恰好 1 成功 1 `insufficient_points`、终态 0、1 条 consume 流水、无负余额**。且**在 MySQL `test_cmhub` 上单独跑过**(progress `ConcurrentDebitTests ... 1 test OK`)——`select_for_update` 在 MySQL 真生效,不是 SQLite 上的假绿。这条是 §8 最关键的证据。
3. **退点幂等双重保障**:锁 `CallRecord` 行序列化并发退点 + 查 REFUND 流水去重;测试断言二次退点 `refunded=False`、REFUND 流水恒为 1 条。
4. **DB 级约束齐全(不只靠 CHECK)**:`user_wallet.points_balance>=0`、`call_record.points_cost>=0`、`points_ledger.balance_after>=0` 且 `points_delta!=0`、`pricing_rule.points_cost>0` + `unique(operation_type,alias,resolution)`、`exchange_rate.points_per_unit>0`——全部落到迁移文件(DB 级),锁 + 约束「带保险的双保险」,符合 §8「不能只靠 CHECK」。
5. **后台无法绕过账本**:`UserWallet` admin 的 `points_balance` **只读**、禁增删 → 运营改不了余额,逼所有余额变动走计费层;`PointsLedger`/`CallRecord` admin 全只读(只追加)。这是 §8「只追加/留痕」在 UI 层的正确收口。
6. **ApiKey 安全存储**:`sk_cmhub_` + `secrets.token_urlsafe(32)`;`sha256` 哈希、`key_prefix` 便于检索、`hmac.compare_digest` 定长比对防时序侧信道;admin `key_hash` 脱敏、禁新增/删除(创建走自助 T-502)。测试断言明文不入 `__dict__`。
7. **计费与底层模型解耦**:`PricingRule(operation_type, alias, resolution)` 定价,换 `ModelAlias→AiModel` 映射不影响价格(专测验证);分辨率精确规则优先、空串 `""` 作 wildcard 兜底;缺规则抛 `NoPricingRuleError(code="no_pricing_rule")`。
8. **金额用 Decimal + floor**:`quote_recharge_points` 用 `Decimal(str(amount)) × points_per_unit` 后 `ROUND_FLOOR`,与「floor(amount×rate)」一致,规避浮点误差;汇率取「active 且 effective_from<=now 的最新一条」。
9. **CallRecord 落实 Phase 1 P3**:无 `raw`/`provider_raw` 字段(专测断言),只有 `result_ref` + `result_summary`,不整包 dump 上游响应。
10. **错误码与 `api.md` 对齐**:`insufficient_points`→402、`no_pricing_rule`→400;`api.md` 已记录 `precharge_call` 用 `select_for_update`、余额不足不建 CallRecord/不写 Ledger/不调上游。
## 三、修补清单
### P1 · 现在改
**无。** 资金核心零 P1 是很好的信号——先扣后调、失败必退、并发锁、只追加留痕、DB 约束全部到位且有真并发测试。以下均为加固/收尾,不涉及正确性缺陷。
### P2 · 建议处理
#### P2-1 退点幂等缺 DB 级兜底约束
- **症状**:退点幂等目前靠「锁 `CallRecord` 行 + 查 REFUND 流水」保证,锁内安全、测试也覆盖。但 `points_ledger` **没有** 一条「每个 `ref_call` 最多一条 REFUND」的 DB 约束做 backstop。若将来某个代码路径**没先锁 `CallRecord`** 就写 refund 流水(例如后台脚本、批量补偿),双退不会被数据库拦住。
- **位置**:`apps/billing/models.py:PointsLedger.Meta.constraints`。
- **怎么改**:不要用 `UniqueConstraint(..., condition=Q(change_type="refund"))` 这类 partial unique;MySQL 不支持通用条件唯一索引,Django 在 MySQL 上无法按预期落地。T-204 采用 MySQL 可执行的普通复合唯一约束:`UniqueConstraint(fields=["ref_call", "change_type"], name="unique_ledger_change_type_per_call")`。这会保证同一 `CallRecord` 最多一条同类型流水;当前业务模型下每次调用只应有一条 `consume` 和最多一条 `refund`,符合账务口径。`ref_call=NULL` 的充值/订单流水在 MySQL 唯一索引下仍可多条存在,不影响充值流水。
- **验证**:迁移生成该复合唯一约束;新增测试:绕过服务层直接连写两条同 `ref_call` 的 REFUND,第二条抛 `IntegrityError`;同时确认同一 `ref_call` 的一条 CONSUME + 一条 REFUND 可以共存。
#### P2-2 全量测试从未一次跑绿(受远程 MySQL 不稳定影响)
- **症状**:因远程 MySQL `43.128.3.240:3306` 间歇超时/重置,完整 `manage.py test`(43 tests)**从未在单次运行里全绿**;codex 靠分 app / 分测试类子集逐一通过来佐证。最关键的并发测试已单独在 MySQL 通过,但缺一次「完整一遍绿灯」,跨测试隔离性未在单次验证。
- **位置**:测试运行环境 / `docs/03-tech-stack.md` 测试说明。
- **怎么改**:在稳定 DB(本地 MySQL 8.4,或修复/更换远程实例)上跑一次完整 `manage.py test` 并把结果记进 `progress.md`;并在测试文档显式标注「**并发测试必须在 MySQL 上跑**,SQLite 无行锁会假绿」,防止将来有人用 SQLite 误判。
- **验证**:`progress.md` 出现一条完整 43+ tests 单次全绿记录。
### P3 · 登记 / 后续任务(不在 T-204 硬性范围)
- **手工调点服务未实现**(§8「运营手工调整必须填原因」):模型层已备好(`PointsLedger` ADJUST 类型的 `clean()` 强制要求 `reason`),wallet admin 已锁死不可直接改余额(安全的中间态);但还缺一个 billing 层 `adjust_points()`(锁 wallet + 写 adjust 流水 + reason)供后台用。→ 登记 **T-401**。
- **wallet 在扣点热路径 `get_or_create`**:`precharge_call` 用 `select_for_update().get_or_create(user=...)`,会在「钱包缺失」时静默建零钱包再报余额不足,掩盖异常。应保证注册(T-501)时建钱包;billing 层遇缺失可考虑显式报错。→ 登记 **T-501**。
- **锁顺序约定**:`refund_call_points` 锁序为 (CallRecord → wallet),`precharge_call` 只锁 wallet。当前无跨序死锁;但将来若新增「先锁 wallet 再锁 CallRecord」路径需警惕死锁。→ 作为约定登记。
- **只追加靠 admin + 无 UPDATE 代码保证**:`PointsLedger`/`CallRecord` 的不可改目前由 admin 只读 + 服务层不写 update 保证,DB 层无触发器阻止 UPDATE/DELETE。Django 层已足够,登记备忘即可。
- **`OperationType` 双份枚举**:`apps.ai.ModelAlias.OperationType` 与 `apps.billing.CallRecord.OperationType` 各定义一份 title/image,语义重复。极小的 DRY 隐患,后续可抽公共枚举。
## 四、说明:未本地复跑
审核机(WSL)无 `python3.12`,**未本地复跑**。本报告基于:静态审查(`users/models`、`billing/models`、`services`、`pricing`、两 app 的 `admin`、`tests`、全部迁移、`settings`)+ codex 执行记录(T-201 34 tests、T-202 billing 10 tests、T-203 `BillingServiceTests` 4 + `ConcurrentDebitTests` 1 均**单独在 MySQL 通过**;完整套件因远程 MySQL 连接问题未单次全绿,失败点均为连接/建库超时而非断言失败,见 P2-2)。
T-204 处理后,请重跑 `check`/`test`/`init` 并把证据记入 `progress.md`(`06-tasks.md` 使用规则第 5 条)。
## 五、T-204 完成定义
- **P2-1** 已处理:`points_ledger` 加 MySQL 可落地的 `ref_call + change_type` 复合唯一约束,作为「每个调用最多一条 REFUND」的 DB 级兜底;禁止使用 MySQL 不支持的 partial unique;补绕服务层直写的 `IntegrityError` 测试,并确认同一调用的 CONSUME 与 REFUND 可共存。
- **P2-2** 已处理:在稳定 MySQL 上跑一次完整 `manage.py test` 全绿并留证;测试文档标注「并发测试须在 MySQL 上跑」。
- **P3** 各项已在 `06-tasks.md` 对应任务(T-401 / T-501)或作为约定/备忘登记,不遗失。
- `check` 0 issues、`test` 全绿、`init` 通过,证据入 `progress.md`。