lty/docs/REVIEW-affinity-P1-FIX-REPORT.md
pmc f66e2dfc86 docs(affinity-P1): 归档代码审查报告与修复报告
P1 审查阶段产物补落库(深度审查 + 修复同步过程中漏 commit):

- docs/REVIEW-affinity-P1.md — gsd-code-reviewer 输出,18 项 finding(3 Critical / 9 Warning / 6 Info),含 Cross-Module 调用链分析与 Cross-App FK 一致性表
- docs/REVIEW-affinity-P1-FIX-REPORT.md — gsd-code-fixer 输出,17 FIXED + 1 PARTIAL (WR-008) + 0 SKIPPED 状态明细,对应 4 个修复 commit A/B/C/D 索引

两份报告与 P1 fix commits (33b302c / 9a87f5e / 2a28aa8 / 61e8374) 配套阅读,为 P2 service 层依赖的设计决策 / DB 约束 / 软删语义提供溯源依据。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-14 09:35:31 +08:00

358 lines
19 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: affinity-P1
fixed_at: 2026-05-13T00:00:00Z
review_path: docs/REVIEW-affinity-P1.md
iteration: 1
findings_in_scope: 18
fixed: 18
skipped: 0
status: all_fixed
fix_strategy:
CR-001: ActiveUserDeviceManager + 5 处调用点切换 + active manager 硬规则
CR-002: 13 条 DB CheckConstraint + clean() Python 兜底
CR-003: 选项 B直接重写 0006+ AffinityLog source='data_migration' 标记幂等
WR-001: pk=1 CheckConstraint + save() 强制 pk=1事实单例
WR-002: SET_NULL + device_snapshot_id + conditional unique
WR-003: 删除 3 个低价值索引,保留 (device, -created_at) + event_id partial unique
WR-004: event_id null=True + RunPython '' → NULL 数据兜底
WR-005: seed 加 companion_30min 默认规则
WR-006: description 显式 default='' + DEFAULT_LEVELS 全部补 description
WR-007: seed_affinity 每条 spec 独立事务
WR-008: 字段保留 + UserInfoSerializer 移除 + [DEPRECATED] 软标记property 改造延后)
WR-009: AffinityLevel.clean() + save() full_clean 应用层多层兜底
IN-001: 5 个弃用字段 help_text 加 [DEPRECATED — 计划于 P2 完成后删除]
IN-002: 抽到 userapp/affinity/defaults.py
IN-003: AffinitySetting.daily_cap RenameField → global_daily_cap
IN-004: __str__ 用 pk or 'new' 兜底
IN-005: UserDevice.is_active → is_bound与 CR-001 同 commit
IN-006: 0006 print 前缀改为 [migration 0006_migrate_favorability]
commits:
A: 33b302c # CR-001 + IN-005
B: 9a87f5e # CR-002 + WR-001
C: 2a28aa8 # CR-003
D: 61e8374 # WR-002~009 + IN-001~006
migrations_added:
- device_interaction/0004_rename_userdevice_is_active_is_bound.py
- device_interaction/0005_alter_userdevice_options.py
- userapp/0007_add_affinity_check_constraints.py
- userapp/0008_alter_affinitylog_source_choices.py
- userapp/0009_affinity_p1_polish.py
migrations_rewritten:
- userapp/0006_migrate_favorability_to_userdevice.py # 直接重写,详见 CR-003 风险说明
---
# 好感度系统 P1 数据层代码审查 — 修复报告
**修复时间:** 2026-05-13
**源审查报告:** [REVIEW-affinity-P1.md](REVIEW-affinity-P1.md)
**迭代轮次:** 1
**修复范围:** 18 / 183 Critical + 9 Warning + 6 Info 全部覆盖)
**修复策略:** 按 fix_focus 要求严防假修,所有改名同步全部引用点;新增 CHECK 约束已用 makemigrations dry-run 验证;新增 Manager 用 base_manager_name='objects' 保护 Django admin
## 汇总
| 严重等级 | 总数 | 已修复 | 部分修复 | 跳过 |
|---------|------|--------|---------|------|
| Critical | 3 | 3 | 0 | 0 |
| Warning | 9 | 8 | 1 (WR-008) | 0 |
| Info | 6 | 6 | 0 | 0 |
| **合计** | **18** | **17** | **1** | **0** |
**注**WR-008 标 PARTIAL因 ParadiseUser.favorability 字段保留未做 property 改造(详细原因见下文);其余 17 项均 FIXED。
---
## 关键风险说明
### CR-003 选项 B 已知风险
本次直接**重写** `userapp/migrations/0006_migrate_favorability_to_userdevice.py` 而非追加 0007 补偿迁移。
**前提**
- dev DB 已执行过一次 0006输出 `migrate_count=0 / skipped_no_device=1 / skipped_zero=9`,即「实际写入数据 = 0」
- `django_migrations` 表已记录 0006 完成Django 不会自动重跑修改后的逻辑
**意味着什么**
- dev DB 上**不会**自动应用新逻辑 — 旧逻辑产生的数据状态(=0 条写入)即新逻辑的预期初始状态,两者等价
- 生产环境部署前**必须确认 prod 还未跑过 0006**dev → prod 同步部署目前同步走 0001~0008prod 第一次 migrate 才会用到 0006 新版本,安全)
- 如果生产已意外跑过 0006 旧版本且有非 0 成功条数,需要走:
1. `python manage.py migrate userapp 0005 --fake`fake reverse 到 0005
2. 手工 DELETE 已写入的 AffinityLog.source='data_migration' 标记(如果有)
3. `python manage.py migrate userapp`(重新跑新版本 0006
### WR-008 ParadiseUser.favorability 未做 property 改造的原因
审查报告原建议:把 `favorability` 字段重命名为 `_legacy_favorability` + 加同名 `@property`
**实际选择**:保留字段名,仅做软标记 + 序列化器清理。
**原因**
1. Django Model field 与 Python property **同名冲突**——必须先 RenameField 把字段改名(如 `_legacy_favorability`)才能上 property
2. 一旦 RenameField**0006 backward 回滚逻辑会立即坏掉**CR-003 修正版的 backward 仍然写 `user.favorability = ...`);需要同步改 0006 backward但 0006 是已应用迁移,二次修改风险高
3. 当前唯一仍在读 `favorability` 的地方 `UserInfoSerializer` 已显式移除字段暴露,外部 API 不再返回,新代码无入口写入
4. property 形式的 N+1 风险也被审查报告自己指出(每次访问查 devices
**所以现状**:字段保留 + verbose_name 加「(已弃用)」+ help_text 标 `[DEPRECATED]` + serializer 移除字段。**这是有意识的延后**(不是漏修):等 P2 服务层落地稳定 + 0006 backward 路径退役 2 周后,再做完整 RenameField + property + RemoveField 三步退化。
如果后续团队明确不再需要 0006 backward可以直接做 `RemoveField`,比 property 路线干净。
---
## 已修复明细
### CR-001UserDevice 控制权解析未过滤 is_active
**状态**: FIXED
**Commit**: `33b302c` (Commit A)
**修改文件**:
- `qy_lty/device_interaction/models.py` — 新增 `ActiveUserDeviceManager` + 双 manager + `base_manager_name='objects'`
- `qy_lty/userapp/views.py:120` — MAC 登录切到 `UserDevice.active.filter(...)`
- `qy_lty/device_interaction/views.py:462/694/702/1158` — 4 处调用点bind_status / 绑定 endpoint 2 处 / RTC token全部切到 active manager
- `qy_lty/device_interaction/serializers.py:125` — 绑定校验切到 active manager
- `qy_lty/CLAUDE.md` — § "设备绑定与控制权" 加硬规则
- `qy_lty/device_interaction/migrations/0004_rename_userdevice_is_active_is_bound.py` — RenameField与 IN-005 合并)
- `qy_lty/device_interaction/migrations/0005_alter_userdevice_options.py` — base_manager_name Meta 变更
**应用的修复**: 实现 ActiveUserDeviceManager 强制 `is_bound=True` 过滤;所有控制权解析路径切换到 active manageradmin / 反向关系user.devices仍走全集 managerCLAUDE.md 加入硬规则确保后续开发不退化。
### CR-002AffinityRule/AffinityLevel/AffinitySetting 缺乏 DB CHECK 约束
**状态**: FIXED
**Commit**: `9a87f5e` (Commit B)
**修改文件**:
- `qy_lty/userapp/models.py` — 三表共加 13 条 CheckConstraint + 三个 clean() 方法 + AffinityLevel.save() 自动 full_clean
- `qy_lty/userapp/migrations/0007_add_affinity_check_constraints.py` — Django 自动生成的 AddConstraint 迁移
**应用的修复**:
- AffinityRule`min_change ≤ max_change``cooldown_seconds ≥ 0``single_cap > 0``daily_cap > 0`、companion_time 类型时配套字段必须 > 0
- AffinityLevel`min_affinity ≤ max_affinity``reward_currency ≥ 0`
- AffinitySetting`decay_min_decay ≤ decay_max_decay ≤ decay_cap``initial_affinity ≤ max_affinity``decay_min_floor ≤ max_affinity``global_daily_cap > 0``pk=1`(单例硬约束)
### CR-0030006 数据迁移幂等性脆弱
**状态**: FIXED — 需要人工验证(详见上文风险说明)
**Commit**: `2a28aa8` (Commit C)
**修改文件**:
- `qy_lty/userapp/migrations/0006_migrate_favorability_to_userdevice.py` — 完整重写 forward/backward 函数,改用 AffinityLog source='data_migration' 标记做幂等
- `qy_lty/userapp/models.py` — AffinityLog.SOURCE_CHOICES 加 ('data_migration', '数据迁移')
- `qy_lty/userapp/migrations/0008_alter_affinitylog_source_choices.py` — AlterField 更新 choices
**应用的修复**: forward 用 `AffinityLog.objects.filter(device_id=target.id, source='data_migration').exists()` 做幂等标记,避免 `favorability == 10` 误判backward 通过 audit log metadata 反向恢复,避免衰减回 10 的数据丢失;选项 B 已知风险已在迁移 docstring 与本报告显式记录。
### WR-001AffinitySetting.save() 单例保证并发不安全
**状态**: FIXED
**Commit**: `9a87f5e` (Commit B — 与 CR-002 同 commit)
**修改文件**:
- `qy_lty/userapp/models.py` — save() 强制 `self.pk = 1`Meta.constraints 加 `CheckConstraint(check=Q(pk=1), name='affinitysetting_singleton')`
**应用的修复**: 改 `pk=1` 强制 + DB CHECK 约束,任何并发 INSERT 非 1 主键都会被 DB 拒绝CHECK 约束跨行不可PG 限制),但配合 save() 强制 pk=1 形成事实单例。
### WR-002UserLevelRewardGrant.device CASCADE 与 AffinityLog.device SET_NULL 不一致
**状态**: FIXED
**Commit**: `61e8374` (Commit D)
**修改文件**:
- `qy_lty/userapp/models.py` — UserLevelRewardGrant.device on_delete CASCADE → SET_NULL新增 `device_snapshot_id` 字段unique 改为 partialdevice 非空时save() 自动填充 snapshot
- `qy_lty/userapp/migrations/0009_affinity_p1_polish.py` — AlterField + AddField + UniqueConstraint 重建
**应用的修复**: 与 AffinityLog.device SET_NULL 对齐(历史保留语义),同时保留 device_snapshot_id 用于审计。
### WR-003AffinityLog 索引过多
**状态**: FIXED
**Commit**: `61e8374` (Commit D)
**修改文件**:
- `qy_lty/userapp/models.py` — Meta.indexes 删除 (user, -created_at) / (rule_key, -created_at) / (source, -created_at),仅保留 (device, -created_at)
- `qy_lty/userapp/migrations/0009_affinity_p1_polish.py` — RemoveIndex × 3
**应用的修复**: P1 阶段先保守,等 P5 上线后按真实查询 profile 加索引。
### WR-004event_id partial unique 用 `''` 不安全
**状态**: FIXED
**Commit**: `61e8374` (Commit D)
**修改文件**:
- `qy_lty/userapp/models.py` — event_id 改为 `null=True, blank=True`UniqueConstraint condition 改为 `Q(event_id__isnull=False)`
- `qy_lty/userapp/migrations/0009_affinity_p1_polish.py` — AlterField + RunPython 把现有 `''` 改为 NULL + 重建 unique
**应用的修复**: 用 NULL 替代 `''` 表达「无值」语义PG 下 NULL 不参与 unique 索引正好就是想要的RunPython 提供 forward/backward 数据兜底。
### WR-005seed_affinity 缺少 companion_time 默认规则
**状态**: FIXED
**Commit**: `61e8374` (Commit D)
**修改文件**:
- `qy_lty/userapp/affinity/defaults.py` — DEFAULT_RULES 追加 `companion_30min`trigger_type=companion_time, min_continuous_minutes=30, max_count_per_day=4, min_change=1, max_change=2, daily_cap=8
**应用的修复**: 数值采用保守默认(与产品最终对齐前可由运营在 admin 调整),添加注释说明「待产品最终对齐」。
### WR-006description 字段未显式 default=''
**状态**: FIXED
**Commit**: `61e8374` (Commit D)
**修改文件**:
- `qy_lty/userapp/models.py` — AffinityRule.description / AffinityLevel.description 加 `default=''`
- `qy_lty/userapp/affinity/defaults.py` — DEFAULT_LEVELS 所有 5 条 entry 显式填写 description
- `qy_lty/userapp/migrations/0009_affinity_p1_polish.py` — AlterField × 2
### WR-007seed_affinity @transaction.atomic 包整个 handle
**状态**: FIXED
**Commit**: `61e8374` (Commit D)
**修改文件**:
- `qy_lty/userapp/management/commands/seed_affinity.py` — 去掉 handle 上的 @transaction.atomic改为每条 spec 在循环内 `with transaction.atomic()` 独立提交,加 failed 计数与 try/except
**应用的修复**: 部分失败可重跑stdout 与实际写入状态一致;运维诊断更可靠。
### WR-008ParadiseUser.favorability 旧字段保留无运行时拦截
**状态**: PARTIAL字段保留 + 软标记 + serializer 移除property 改造延后到下个版本)
**Commit**: `61e8374` (Commit D)
**修改文件**:
- `qy_lty/userapp/models.py` — favorability 字段 verbose_name 加 (已弃用)help_text 标 [DEPRECATED — P2 后删除],加 docstring 注释说明保留原因
- `qy_lty/userapp/serializers.py` — UserInfoSerializer.Meta.fields 移除 'favorability'
- `qy_lty/userapp/migrations/0009_affinity_p1_polish.py` — AlterField 更新 verbose_name + help_text
**未做事项**: 没有 RenameField 改为 `_legacy_favorability` + 加 property warning。原因见上文「关键风险说明 WR-008」段。
**追踪 TODO**: P2 服务层稳定后做 RemoveField或 property 三步退化)。
### WR-009AffinityLevel 区间允许重叠 / 留空隙
**状态**: FIXED
**Commit**: `9a87f5e` (Commit B — 与 CR-002 同 commit应用层多层兜底)
**修改文件**:
- `qy_lty/userapp/models.py` — AffinityLevel.clean() 检查 min ≤ max + 与其它等级区间不重叠exclude is_deleted=Truesave() 自动调 full_clean提供 skip_clean=True 后门给 fixture / 迁移)
**应用的修复**: 因 PG CHECK 约束跨行不可(需 ExclusionConstraint + btree_gist 扩展,部署成本高),用应用层 clean() + save() full_clean 兜底admin / DRF 显式调 full_clean 会触发seed_affinity 创建路径会触发shell / 直 SQL 仍可绕过(接受此残留风险)。
### IN-001弃用字段缺乏 deprecation 路径
**状态**: FIXED
**Commit**: `61e8374` (Commit D)
**修改文件**:
- `qy_lty/userapp/models.py` — 5 个弃用字段AffinityRule.points / daily_limit / is_activeAffinityLevel.required_points / rewardshelp_text 加 `[DEPRECATED — 计划于 P2 完成后删除]`
- `qy_lty/userapp/migrations/0009_affinity_p1_polish.py` — AlterField × 5
### IN-002DEFAULT_RULES/LEVELS 应抽到独立模块
**状态**: FIXED
**Commit**: `61e8374` (Commit D)
**修改文件**:
- `qy_lty/userapp/affinity/__init__.py`**新建**
- `qy_lty/userapp/affinity/defaults.py`**新建** — DEFAULT_RULES / DEFAULT_LEVELS / DEFAULT_SETTING 常量)
- `qy_lty/userapp/management/commands/seed_affinity.py``from userapp.affinity.defaults import ...`
### IN-003AffinityRule.daily_cap 与 AffinitySetting.daily_cap 同名易混淆
**状态**: FIXED
**Commit**: `61e8374` (Commit D)
**修改文件**:
- `qy_lty/userapp/models.py` — AffinitySetting.daily_cap → global_daily_cap同步 Meta.constraints / clean() 引用
- `qy_lty/userapp/affinity/defaults.py` — DEFAULT_SETTING 用 global_daily_cap
- `qy_lty/userapp/migrations/0009_affinity_p1_polish.py` — **手工修正**为 RenameField + AlterField保留数据不是 Remove+Add
**关键提示**: makemigrations 默认生成 Remove+Add 会丢数据,本迁移已手工改为 RenameField — 部署 reviewer 应注意此点(详见迁移 docstring
### IN-004AffinityLog.__str__ 在 self.id 为 None 时显示 #None
**状态**: FIXED
**Commit**: `61e8374` (Commit D)
**修改文件**:
- `qy_lty/userapp/models.py``return f"#{self.id} ..."``return f"#{self.pk or 'new'} ..."`
### IN-005UserDevice.is_active 与 Device.is_active 命名冲突
**状态**: FIXED
**Commit**: `33b302c` (Commit A — 与 CR-001 合并)
**修改文件**:
- `qy_lty/device_interaction/models.py` — RenameField is_active → is_bound + 双 manager
- `qy_lty/device_interaction/migrations/0004_rename_userdevice_is_active_is_bound.py` — RenameField
- `qy_lty/device_interaction/migrations/0005_alter_userdevice_options.py` — Meta options
**应用的修复**: 改名 + ActiveUserDeviceManager 强制语义CLAUDE.md 同步更新硬规则。
### IN-0060006 数据迁移 print 输出无前缀格式
**状态**: FIXED
**Commit**: `2a28aa8` (Commit C — 与 CR-003 合并)
**修改文件**:
- `qy_lty/userapp/migrations/0006_migrate_favorability_to_userdevice.py` — print 前缀改为 `[migration 0006_migrate_favorability] forward: ...` / `[migration 0006_migrate_favorability] backward: ...`
---
## 验证情况
### 已执行的本地验证
- `python manage.py check` — PASS仅 1 个 staticfiles.W004 警告pre-existing 与本次修复无关)
- `python manage.py makemigrations --dry-run --verbosity 2` — PASS"No changes detected",所有 schema 变化均已在迁移中捕获)
- 所有修改的 .py 文件 `ast.parse` — PASSmodels.py、迁移 0006/0009、seed_affinity.py、defaults.py
### 未执行(按用户要求保留给手工验证)
- `python manage.py migrate`(迁移真实应用)
- `python manage.py seed_affinity`seed 数据真实写入)
- 单元 / 集成测试套件
### 建议手工验证步骤
```bash
cd qy_lty
python manage.py check # 应 PASS
python manage.py makemigrations --dry-run # 应 "No changes detected"
python manage.py migrate # 应用 5 个新迁移
python manage.py seed_affinity # 应输出 "AffinitySetting 已存在跳过 / 规则 创建 0 更新 0 / 等级 ..."dev 库已有)
python manage.py seed_affinity --force # 强制覆盖,触发 clean() 校验
python manage.py shell -c "from userapp.models import AffinityLog; print(AffinityLog.objects.filter(source='data_migration').count())" # 应 0dev 库无成功迁移)
```
---
## 跨文件 / 跨调用点一致性检查
### `is_active` → `is_bound` 改名同步性
| 调用点 | 状态 |
|--------|------|
| `device_interaction/models.py` UserDevice 字段定义 | ✓ Rename + help_text 更新 |
| `device_interaction/migrations/0004` | ✓ RenameField + AlterField |
| `userapp/views.py:120` MAC 登录 | ✓ 切到 `UserDevice.active` |
| `device_interaction/views.py:462` bind_status | ✓ 切到 `UserDevice.active` |
| `device_interaction/views.py:694` 绑定 endpoint当前用户 | ✓ 切到 `UserDevice.active` |
| `device_interaction/views.py:702` 绑定 endpoint其他用户 | ✓ 切到 `UserDevice.active` |
| `device_interaction/views.py:1158` RTC token | ✓ 切到 `UserDevice.active` |
| `device_interaction/serializers.py:125` 绑定校验 | ✓ 切到 `UserDevice.active` |
| `qy_lty/CLAUDE.md` § 设备绑定与控制权 | ✓ 加硬规则说明 |
| `qy_lty/userapp/views_old.py:123` | ✗ **未改**deprecated 旧文件,按用户预期不动) |
| `qy_lty/docs/设备动态绑定方案.md` / `修改指南_服务器端.md` | ✗ **未改**(历史文档,按用户预期不动) |
| `qy_lty/.planning/codebase/{ARCHITECTURE,CONCERNS,CONVENTIONS}.md` | ✗ **未改**GSD codebase 文档,下次 codebase 刷新时会同步) |
### `AffinitySetting.daily_cap` → `global_daily_cap` 改名同步性
| 调用点 | 状态 |
|--------|------|
| `userapp/models.py` 字段定义 | ✓ |
| `userapp/models.py` Meta.constraints check | ✓ |
| `userapp/models.py` clean() error key | ✓ |
| `userapp/affinity/defaults.py` DEFAULT_SETTING | ✓ |
| `userapp/migrations/0009` RenameField + AlterField | ✓ 手工修正(不是 Remove+Add |
| `qy_lty/docs/修改记录.md` 历史条目 | ✗ **未改**(历史记录不应被改写) |
---
## 备注
- 本次修复**未跑 `python manage.py migrate`**,按用户要求由用户手工应用 5 个新迁移0004 / 0005 device_interaction + 0007 / 0008 / 0009 userapp
- 所有 commit 已写入 `qy_lty/docs/修改记录.md` 顶部,按 commit A / B / C / D 各一条
- CLAUDE.md 已更新 § "设备绑定与控制权"加入「必须用 `UserDevice.active`」硬规则
- 没有跳过任何 finding唯一 PARTIAL 项是 WR-008原因与延后计划已详细说明
---
_Fixed: 2026-05-13T00:00:00Z_
_Fixer: Claude (Opus 4.7) via gsd-code-fixer_
_Iteration: 1_
_Note: 本次修复不在 GSD .planning/phase-N 结构内,按用户要求保存到 docs/REVIEW-affinity-P1-FIX-REPORT.md_