lty/docs/REVIEW-affinity-P1.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

926 lines
39 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
reviewed: 2026-05-13T00:00:00Z
depth: deep
files_reviewed: 6
files_reviewed_list:
- qy_lty/userapp/models.py
- qy_lty/device_interaction/models.py
- qy_lty/userapp/migrations/0005_affinitysetting_affinitylevel_is_deleted_and_more.py
- qy_lty/userapp/migrations/0006_migrate_favorability_to_userdevice.py
- qy_lty/device_interaction/migrations/0003_userdevice_affinity_level_userdevice_favorability_and_more.py
- qy_lty/userapp/management/commands/seed_affinity.py
findings:
critical: 3
warning: 9
info: 6
total: 18
status: issues_found
---
# 好感度系统 P1 数据层代码审查报告
**审查时间:** 2026-05-13
**审查深度:** deep含 cross-app FK 与 cross-module 调用链分析)
**审查范围:** P1 阶段P1-01 ~ P1-10数据层产出物共 6 个文件
**状态:** issues_found3 critical / 9 warning / 6 info
## Summary
P1 阶段建模整体方向是合理的:把好感度从 `ParadiseUser` 下沉到 `UserDevice` 是符合"一人多设备 / 设备级人格"业务语义的正确选择;规则 / 等级 / 日志 / 计数器 / 奖励发放记录五张表的拆分清晰;冗余 `rule_key` 字段、`event_id` 部分唯一索引、`reward_snapshot` JSON 快照等都是经验性的好实践。
但仍存在三类共 18 个需要在 P2 服务层动工之前修复的问题:
1. **完整性约束严重缺失**Critical`AffinityRule` 没有任何 DB 级 CHECK 约束保证 `min_change <= max_change``cooldown_seconds >= 0``single_cap > 0``daily_cap > 0``AffinitySetting` 没保证 `decay_min_decay <= decay_max_decay <= decay_cap``AffinityLevel` 没保证 `min_affinity <= max_affinity` 且区间不重叠。一旦管理后台 / Admin / shell 写入异常值P2 服务层的 `random.randint(min, max)` 会直接抛 `ValueError`,日上限会永远命中或永远不命中,是线上爆炸级风险。
2. **既有"换绑挤掉旧绑定"语义被 `is_active` 新字段悄悄破坏**Critical现有 4 处 `UserDevice.objects.filter(device=...).order_by('-bound_at').first()` 调用点(`userapp/views.py:120``device_interaction/views.py:462/1158``serializers.py:125`)都没有过滤 `is_active=True`。一旦 P2 把解绑实现为软删(设 `is_active=False`),这些已经上线的代码会继续把旧的、已失效的绑定者当作"最新绑定者"返回,导致 MAC 登录拿到错误的 user-token、WS 分组路由到错误的 `device_{user_id}`。这是直接破坏运行中功能的语义回归。
3. **0006 数据迁移幂等性建立在脆弱的语义假设上**Critical迁移用 `target.favorability == 10` 来判定"未迁移过",但 10 既是初始值、也是衰减下限附近的常见值、也可能被管理员设为 10。重跑迁移会再次覆盖任何当前值正好为 10 的合法记录。回滚函数同样依赖 `!= 10` 判定,导致一旦正常运行了一段时间后再回滚,所有衰减回 10 的设备数据都不会被还原。
其它问题包括:`AffinitySetting.save()` 在并发下仍可能制造重复行、`UserAffinityDailyCounter``UserLevelRewardGrant``CASCADE``AffinityLog``SET_NULL` on_delete 行为不一致、`seed_affinity` 命令缺少 `companion_time` 类规则、整个 `handle` 包一个事务导致 force 模式的部分失败影响放大、`AffinityLevel` 区间没有 DB 唯一约束允许重叠区间、旧字段 `ParadiseUser.favorability` 没有标 deprecation 没有运行时拦截。
下文按严重性详列。修复优先级建议:**先解决 3 个 Critical必须阻塞 P2→ 再处理 9 个 Warning建议在 P2 第一周完成)→ Info 可与 P2 服务层一起渐进改造**。
---
## Critical Issues
### CR-001UserDevice 既有"最新绑定者"取数逻辑未过滤 is_active导致软删后控制权解析错乱
**File:**
- `qy_lty/userapp/views.py:120`MAC 登录)
- `qy_lty/device_interaction/views.py:462``bind_status`
- `qy_lty/device_interaction/views.py:1158`RTC token
- `qy_lty/device_interaction/serializers.py:125`(绑定校验)
- 上述行为依赖 `qy_lty/device_interaction/models.py:122-126` 新增的 `is_active` 字段
**Issue:**
`UserDevice` 在 P1 引入了 `is_active` 字段(设计文档说"解绑置 false重绑时可读取历史值"),但既有的 4 处"换绑挤掉旧绑定"调用点全部使用裸 `filter(device=device)``.order_by('-bound_at').first()` 形式,**没有过滤 `is_active=True`**
```python
# userapp/views.py:120
user_device = UserDevice.objects.filter(device=device).order_by('-bound_at').first()
# device_interaction/views.py:462
user_device = UserDevice.objects.filter(device=device).first() # 隐式依赖 Meta.ordering
# device_interaction/serializers.py:125
if value != 'AA:BB:CC:DD:EE:FF' and UserDevice.objects.filter(device=device).exists():
```
CLAUDE.md §"设备绑定与控制权"显式声明这是当前的**控制权解析规则**——同一台设备同一时刻只有最近一次绑定的那个用户能控制它。一旦 P2 把"解绑"实现为软删(设 `is_active=False`),这些已经上线的代码路径会继续把已失效的旧绑定者当作"最新绑定者"返回,造成:
- MAC 登录端点签发**已解绑用户**的 user-token认证泄露
- WebSocket 分组 `device_{user_id}` 路由到**已解绑用户**的频道(设备消息被前主人收到)
- RTC `room_id = room_{user_id}` 同上
- 重新绑定时 `UserDevice.objects.filter(device=device).exists()` 永远为 True新用户永远绑不上
**Fix:**
在 P2 实现软删之前,先在所有 4 处调用点(以及未来新增的查询)显式加 `is_active=True` 过滤。建议增加一个 manager 强制:
```python
# qy_lty/device_interaction/models.py
class ActiveUserDeviceManager(models.Manager):
def get_queryset(self):
return super().get_queryset().filter(is_active=True)
class UserDevice(models.Model):
# ...
objects = models.Manager() # 默认 manager保留访问历史记录
active = ActiveUserDeviceManager() # 仅查询有效绑定
```
然后改写所有调用点为 `UserDevice.active.filter(device=device).order_by('-bound_at').first()`。同步更新 `qy_lty/CLAUDE.md` §"设备绑定与控制权"、`qy_lty/.planning/codebase/CONVENTIONS.md` 把"必须过滤 is_active"写成硬规则。
**影响范围:**
- 阻塞 P2 任何"解绑 = 软删"的设计
- 影响所有现存的 MAC 登录、WS 分组、RTC 房间路由
- 直接的安全 / 越权风险
---
### CR-002AffinityRule / AffinityLevel / AffinitySetting 缺乏 DB 级 CHECK 约束service 层会被脏数据击穿
**File:**
- `qy_lty/userapp/models.py:79-171``AffinityRule`
- `qy_lty/userapp/models.py:174-244``AffinityLevel`
- `qy_lty/userapp/models.py:247-314``AffinitySetting`
**Issue:**
P2 服务层一定会做形如 `random.randint(rule.min_change, rule.max_change)``if today_total >= rule.daily_cap` 这样的运算。但当前模型对管理后台 / Admin / `python manage.py shell` / API 写入完全没有 DB 级保护:
1. `min_change > max_change``random.randint``ValueError`,整个 apply 流程崩溃
2. `cooldown_seconds < 0` → 冷却计算 `last_trigger + timedelta(seconds=cooldown)` 得到过去时刻,永久解锁
3. `single_cap <= 0` → 任何变化值都被钳为 0
4. `daily_cap <= 0` → 第一次写入就触发上限
5. `AffinitySetting.decay_min_decay > decay_max_decay` → 衰减任务崩
6. `AffinitySetting.max_affinity < initial_affinity` → 新设备初始好感度已超上限
7. `AffinityLevel.min_affinity > max_affinity` → 该等级永远不触发
8. 多个 `AffinityLevel` 的区间可以**重叠**(如 Lv2 [21,40] 与 Lv3 [35,60]),服务端等级匹配结果取决于查询顺序,不确定
9. `AffinityLevel` 区间可以**留空隙**(如 Lv2 [21,40]、Lv3 [50,60]),好感度 45 的设备无等级
App 层校验form / serializer / clean()只是第一道防线shell / 直 SQL / data migration / 第三方导入都能绕开。
**Fix:**
增加一个新迁移 `0007_add_affinity_check_constraints.py`,加 PostgreSQL CHECK 约束。Django 5 用 `models.CheckConstraint`
```python
# AffinityRule.Meta
constraints = [
models.CheckConstraint(
check=Q(min_change__lte=F('max_change')),
name='affinityrule_min_le_max',
),
models.CheckConstraint(
check=Q(cooldown_seconds__gte=0),
name='affinityrule_cooldown_nonneg',
),
models.CheckConstraint(
check=Q(single_cap__gt=0),
name='affinityrule_single_cap_positive',
),
models.CheckConstraint(
check=Q(daily_cap__gt=0),
name='affinityrule_daily_cap_positive',
),
# companion_time 类型时 min_continuous_minutes / max_count_per_day 必须非空
models.CheckConstraint(
check=(~Q(trigger_type='companion_time')) |
(Q(min_continuous_minutes__gt=0) & Q(max_count_per_day__gt=0)),
name='affinityrule_companion_fields_present',
),
]
# AffinityLevel.Meta
constraints = [
models.CheckConstraint(
check=Q(min_affinity__lte=F('max_affinity')),
name='affinitylevel_min_le_max',
),
models.CheckConstraint(
check=Q(reward_currency__gte=0),
name='affinitylevel_currency_nonneg',
),
]
# AffinitySetting.Meta
constraints = [
models.CheckConstraint(
check=Q(decay_min_decay__lte=F('decay_max_decay')),
name='affinitysetting_decay_min_le_max',
),
models.CheckConstraint(
check=Q(decay_max_decay__lte=F('decay_cap')),
name='affinitysetting_decay_within_cap',
),
models.CheckConstraint(
check=Q(initial_affinity__lte=F('max_affinity')),
name='affinitysetting_initial_le_max',
),
models.CheckConstraint(
check=Q(decay_min_floor__lte=F('max_affinity')),
name='affinitysetting_floor_le_max',
),
]
```
同时在模型 `clean()` 加 Python 级校验,并在管理后台 / DRF serializer 显式 `full_clean()`,给前端友好错误信息。
**等级区间不重叠 / 不留空隙**用 DB 约束实现成本极高(需要 exclusion constraint + btree_gist建议保留在应用层`AffinityLevel.clean()` 检查 `min_affinity` 等于上一级 `max_affinity + 1`,并在 `seed_affinity` 验证完整覆盖 `[0, max_affinity]`
**影响范围:**
- 阻塞 P2 服务层(否则随时会因为脏配置崩溃)
- 数据完整性、运维侧的可观测性
- 管理后台需要补充对应的前端表单校验
---
### CR-0030006 数据迁移用 `target.favorability == 10` 做幂等条件,存在数据覆盖与回滚失效双重风险
**File:** `qy_lty/userapp/migrations/0006_migrate_favorability_to_userdevice.py:54-58, 67-85`
**Issue:**
forward 函数的幂等条件:
```python
if target.favorability == 10: # 行 55
target.favorability = favorability
target.save(update_fields=['favorability'])
migrated_count += 1
```
backward 函数的回滚条件:
```python
if primary and primary.favorability != 10: # 行 80
user.favorability = primary.favorability
user.save(update_fields=['favorability'])
```
10 是 `UserDevice.favorability` 字段的 model 默认值,也是 `AffinitySetting.initial_affinity` 的默认值,意味着:
1. **未来某用户合法地把好感度衰减到 10**(衰减任务跑了一段时间后),如果运维误执行 `migrate --fake` 后再 `unfake` 或重做某次回滚 / 重跑forward 会**再次把 ParadiseUser.favorability 旧值(很可能是迁移时的快照)写回**,覆盖正常衰减后的当前值,造成业务数据错乱
2. **管理员手工把好感度调整为 10**,下一次重跑迁移同样会被覆盖
3. **backward 在系统稳定运行一段时间后执行**,由于 forward 早已完成,所有非 10 的设备会被回写到 `ParadiseUser.favorability`——但 `ParadiseUser.favorability` 此时是过期数据(业务代码已经全部走 `UserDevice`),回写没有意义;同时所有当前正好为 10 的设备会**被跳过回写**,导致迁移前那批好感度 = 10 的用户**永久丢失**原 favorability 值(如果他们在 forward 时本来有非零 ParadiseUser.favorability 但被跳过)
4. 用户提到迁移已经在 makemigrations 后第二次启动时跑过一次,输出"9 个零值跳过 + 1 个无设备跳过 + 0 成功"——这意味着实际成功迁移条数为 0`MigrationRecorder` 已记录迁移完成。如果以后再有用户从老库恢复数据FK fixture / dumpdata无法再次自动迁移
**Fix:**
方案 A推荐引入幂等标记`AffinityLog` 或 metadata 标记已迁移:
```python
def migrate_favorability_forward(apps, schema_editor):
ParadiseUser = apps.get_model('userapp', 'ParadiseUser')
UserDevice = apps.get_model('device_interaction', 'UserDevice')
AffinityLog = apps.get_model('userapp', 'AffinityLog')
for user in ParadiseUser.objects.iterator():
favorability = getattr(user, 'favorability', 0) or 0
if favorability <= 0:
continue
target = (
UserDevice.objects.filter(user_id=user.id, is_primary=True)
.order_by('-bound_at').first()
or UserDevice.objects.filter(user_id=user.id)
.order_by('-bound_at').first()
)
if target is None:
continue
# 用 AffinityLog 中是否存在 source='data_migration' 的记录做幂等
already = AffinityLog.objects.filter(
device_id=target.id, source='data_migration'
).exists()
if already:
continue
before = target.favorability
target.favorability = favorability
target.save(update_fields=['favorability'])
AffinityLog.objects.create(
user_id=user.id, device_id=target.id,
rule_key='', change_value=favorability - before,
before_value=before, after_value=favorability,
source='data_migration',
metadata={'migration': '0006', 'from_user_favorability': favorability},
)
```
注意要先把 `'data_migration'` 加入 `AffinityLog.SOURCE_CHOICES`
方案 B最小改动`UserDevice` 新增临时字段 `_migrated_from_user_favorability`bool default Falseforward 写入时设为 True幂等判断改为 `if not target._migrated_from_user_favorability`。完成后用单独迁移删字段。
方案 C最低成本直接接受迁移**不可重跑**,把 forward 改为**判断 `MigrationRecorder` 状态**:若 `0006` 已经在 `django_migrations` 表中存在则 noop。这是 Django 迁移系统的默认语义,但需要在 `RunPython` 内显式检查,避免人为 `--fake` 后再 `unfake` 触发。
backward 同样需要重写:不应依赖 `!= 10`,而应使用 forward 写入的 metadata 反向查询。
**影响范围:**
- 重跑迁移会覆盖正常业务数据
- 回滚语义不可靠
- P3 / P4 衰减跑起来后这个迁移会变成"定时炸弹"
---
## Warnings
### WR-001AffinitySetting.save() 单例保证在并发下仍可能制造重复行
**File:** `qy_lty/userapp/models.py:303-308`
**Issue:**
```python
def save(self, *args, **kwargs):
if not self.pk and AffinitySetting.objects.exists():
existing = AffinitySetting.objects.first()
self.pk = existing.pk
super().save(*args, **kwargs)
```
在多 workergunicorn / daphne 多进程)或并发 admin 操作下,两个进程的 `AffinitySetting.objects.exists()` 调用可以同时返回 `False`(首次部署 / 表为空时),随后两个 `INSERT` 都成功,得到两条记录。即便表不为空,`AffinitySetting.objects.first()` 读到的两个 pk 可能不同(理论上单例表不存在这种情况,但代码不应假设),随后两个 `UPDATE` 都成功——总体上单例保证是 best-effort 的,不是强约束。
**Fix:**
1.`get_solo()` 改为 `pk=1` 硬编码 + `update_or_create`
```python
@classmethod
def get_solo(cls):
instance, _ = cls.objects.get_or_create(pk=1, defaults={})
return instance
def save(self, *args, **kwargs):
# 强制 pk=1 单例
if not self.pk:
self.pk = 1
super().save(*args, **kwargs)
```
2. 同时加 DB 级 CHECK 约束防御:
```python
constraints = [
models.CheckConstraint(check=Q(pk=1), name='affinitysetting_singleton'),
]
```
虽然 CHECK 约束在 PostgreSQL 下不能跨行验证(不能强制"全表只有 1 行"),但 `pk=1` 约束可以阻止任何非 1 的主键,配合应用层 `force pk=1` 就能形成事实单例。
**影响范围:**
- 影响低(首次部署窗口期),但配置型数据出现重复行后续排查极痛苦
---
### WR-002UserAffinityDailyCounter / UserLevelRewardGrant on_delete 与 AffinityLog 不一致
**File:**
- `qy_lty/userapp/models.py:339`AffinityLog.device → SET_NULL
- `qy_lty/userapp/models.py:415`UserAffinityDailyCounter.device → CASCADE
- `qy_lty/userapp/models.py:453`UserLevelRewardGrant.device → CASCADE
**Issue:**
设计意图明确是"AffinityLog 历史保留"(已注释"rule 用 SET_NULL规则被删除后日志保留"),但 `UserDevice` 一旦被硬删,`UserLevelRewardGrant` 会跟着级联删除——这破坏了**奖励发放的永久幂等性**:如果未来某天用户重新绑定该设备(虽然现在的设计是 UserDevice 软删,但 Device 本身仍可能被运营删除),原来的发放记录会消失,重新发奖会再次成立。
`UserAffinityDailyCounter` CASCADE 是合理的(计数器本就是每日重置 + Redis 兜底),但 `UserLevelRewardGrant` 应该和 `AffinityLog` 保持一致 → `SET_NULL`device 可空)或 `PROTECT`
**Fix:**
```python
# UserLevelRewardGrant
device = models.ForeignKey(
'device_interaction.UserDevice', on_delete=models.SET_NULL,
verbose_name='用户设备绑定', related_name='level_reward_grants',
null=True, blank=True,
)
```
同时把 `unique_together = [('device', 'level')]` 改为只在 device 非空时唯一(用 `UniqueConstraint(condition=Q(device__isnull=False))`),并冗余一份 `device_snapshot_id`IntegerField, null=True保存被删 device 的原 pk 以便审计。
如果产品 / 运营明确说"Device 删除 = 奖励历史就该清空",那要把这个决策写到设计文档里,并在两处保持一致:`AffinityLog.device` 也要改成 CASCADE。当前两边不一致明显是没对齐设计意图。
**影响范围:**
- 数据保留语义不一致
- 审计 / 合规风险
---
### WR-003AffinityLog 索引设计偏重,写入开销可能放大 5 倍
**File:** `qy_lty/userapp/models.py:387-399`
**Issue:**
当前定义了 6 个索引:
- `created_at` (db_index=True单字段)
- `event_id` (db_index=True单字段)
- `(device, -created_at)` 复合
- `(user, -created_at)` 复合
- `(rule_key, -created_at)` 复合
- `(source, -created_at)` 复合
- `unique_affinity_event_id` partial unique
P2 后端服务每次 apply 都会写 1 条 AffinityLog热点写场景下每行写入需要更新 7 个 B-tree 节点(含主键),写放大显著。
实际查询模式(参考设计文档 §7 客户端接口、§9 管理后台)只用到:
- **客户端拉取最近变化**`(device, -created_at)` 命中
- **管理后台日志列表**:可能按 user / source / rule_key 过滤
- **幂等去重**`event_id` 命中
`(user, -created_at)` 实质冗余——用户视角的查询可以走 `device` 表 join 实现(且数据量比 device 维度大);`(source, -created_at)` 主要用于管理后台筛选,可以用 `created_at` 单字段索引 + source 过滤实现PG 在低基数列上即便走 seq scan + index 也不会太慢。
**Fix:**
P1 阶段建议先保留 `(device, -created_at)``event_id` partial unique、`created_at` 单字段;删除 `(user, -created_at)``(rule_key, -created_at)``(source, -created_at)`。等 P5 上线、有真实查询 profile 后再按需加索引。如果坚持保留管理后台过滤,建议改为 PostgreSQL BRIN 索引(对 `created_at` 等只增字段非常高效):
```python
from django.contrib.postgres.indexes import BrinIndex
indexes = [
models.Index(fields=['device', '-created_at']),
BrinIndex(fields=['created_at']),
]
```
**影响范围:**
- 写入吞吐
- 表空间占用
---
### WR-004unique_affinity_event_id 部分唯一索引语义有歧义,空字符串作为"无 event_id"标记不安全
**File:** `qy_lty/userapp/models.py:393-399`
**Issue:**
```python
constraints = [
UniqueConstraint(
fields=['event_id'],
condition=Q(event_id__gt=''),
name='unique_affinity_event_id',
),
]
```
PostgreSQL 下这会生成 `CREATE UNIQUE INDEX ... WHERE event_id > ''`,是合法的 partial index。但有两个问题
1. **`event_id__gt=''` 语义脆弱**:依赖 `event_id` 是字符串、依赖 `''` 是 "无值"标记。如果未来某客户端 bug 传了 `' '`(空格)或 `'null'`,会被当作有效 event_id 进入唯一约束。
2. **CharField 默认值缺失**`event_id = models.CharField(..., blank=True, db_index=True)`,没有显式 `default=''`。Django 在 IntegerField/CharField 上 blank=True 不等同于 default=''admin 直接保存可能存为 `None`(虽然 CharField 默认 null=False但 raw SQL / fixtures 可能注入 NULL`event_id__gt=''` 对 NULL 不命中,约束失效。
更标准的做法是用 `null=True, blank=True` 配合 `condition=Q(event_id__isnull=False)`
**Fix:**
```python
event_id = models.CharField(
'事件ID', max_length=64, null=True, blank=True, db_index=True,
help_text='客户端事件 UUID用于幂等去重NULL 表示非客户端来源(衰减/管理员调整)',
)
class Meta:
constraints = [
UniqueConstraint(
fields=['event_id'],
condition=Q(event_id__isnull=False),
name='unique_affinity_event_id',
),
]
```
PostgreSQL 下 NULL 不参与 unique 索引,正好就是想要的语义;且 NULL 比 `''` 更明确地表达"无值"。
同步在 P2 服务层加输入校验:长度 < 16 或不符合 UUID 格式的 event_id 一律拒绝并报错
**影响范围:**
- 幂等去重的健壮性
- 兼容性migration 需要 `RunPython` 把现有 `event_id=''` 改成 NULL
---
### WR-005seed_affinity 缺少 trigger_type='companion_time' 类规则,但模型已支持
**File:** `qy_lty/userapp/management/commands/seed_affinity.py:28-77`
**Issue:**
`AffinityRule` 模型在 P1-02 增加了 `trigger_type='companion_time'` 选项与 `min_continuous_minutes` / `max_count_per_day` 配套字段设计文档 §4.2 表格中应有"陪伴时长"规则 30 分钟 +1每日最多 4 次之类 `DEFAULT_RULES` 8 条中 7 条是 `action`1 条是 `decay`**0 companion_time**。
后续 P3 服务层如果按"规则配置驱动"实现陪伴时长检测运行时会**找不到任何 companion_time 规则**要么静默忽略要么报错
**Fix:**
```python
# DEFAULT_RULES 追加
{
'rule_key': 'companion_30min', 'name': '陪伴 30 分钟',
'description': '与洛天依持续陪伴 30 分钟可获得好感度',
'trigger_type': 'companion_time',
'min_change': 1, 'max_change': 2, 'single_cap': 2, 'daily_cap': 8,
'cooldown_seconds': 0, 'is_negative': False, 'is_enabled': True,
'min_continuous_minutes': 30, 'max_count_per_day': 4,
},
```
具体数值需要和产品 / 设计文档对齐如果设计文档暂未明确应在 seed 中加注释说明"待定参见 §4.2"。
另外建议把 DEFAULT_RULES 抽到 `qy_lty/userapp/affinity/defaults.py`避免 management command 文件膨胀且方便单元测试引用
**影响范围:**
- P3 陪伴时长功能阻塞
- 测试覆盖率
---
### WR-006AffinityLevel.description 在 seed 中未设置,依赖 model `blank=True` 隐式 '';未来字段改为 required 会一次性报错
**File:**
- `qy_lty/userapp/management/commands/seed_affinity.py:81-112`
- `qy_lty/userapp/models.py:196` (`description = models.TextField('等级描述', blank=True)`)
**Issue:**
当前 `AffinityLevel.description` model `blank=True` **没有 default**Django `create(**spec)` 时如果 spec 没有 `description` 会在 Python 层报错除非 PostgreSQL 那侧字段允许 NULL TextField 默认 NOT NULL)。
实测时为什么没崩因为 Django 5 TextField + `blank=True` Python 层会注入 `''` 作为隐式默认值但这是 Django 内部行为不是合约一旦未来把 description 改成 required`blank=False`或加 uniqueseed 会一次性失败
**Fix:**
显式补齐
```python
DEFAULT_LEVELS = [
{
'level': 1, 'name': '初识',
'description': '初次相识阶段,了解彼此的基础阶段', # 显式
# ...
},
...
]
```
模型侧也建议补 `default=''`
```python
description = models.TextField('等级描述', blank=True, default='')
```
`AffinityRule.description` 同样问题
**影响范围:**
- 健壮性
- 未来 schema 演进
---
### WR-007seed_affinity @transaction.atomic 包整个 handleforce 模式下部分失败会回滚所有已处理项
**File:** `qy_lty/userapp/management/commands/seed_affinity.py:124-136`
**Issue:**
```python
@transaction.atomic
def handle(self, *args, **options):
force = options['force']
self._seed_setting()
rules_created, rules_updated = self._seed_rules(force)
levels_created, levels_updated = self._seed_levels(force)
self.stdout.write(self.style.SUCCESS(f'\n[seed_affinity] 完成:...'))
```
如果 force 模式下处理第 6 rule 时崩溃例如 JSON 字段格式错 5 rule update 全部回滚 stdout 已经打印 `~ 规则 xxx 已覆盖` 5 造成"显示成功但实际未生效"的语义错位运维排查时会被误导
**Fix:**
两种方案
方案 A推荐每条独立事务
```python
def _seed_rules(self, force):
created, updated = 0, 0
for spec in DEFAULT_RULES:
try:
with transaction.atomic():
# ... 单条处理
except Exception as e:
self.stderr.write(self.style.ERROR(f' ! 规则 {spec["rule_key"]} 处理失败: {e}'))
continue
return created, updated
```
并去掉 `handle` 上的 `@transaction.atomic`这样部分失败不影响其他规则
方案 B保留全局事务但把 stdout 改为推迟输出
```python
def handle(self, *args, **options):
force = options['force']
messages = []
try:
with transaction.atomic():
self._seed_setting(messages)
self._seed_rules(force, messages)
self._seed_levels(force, messages)
except Exception:
self.stderr.write(self.style.ERROR('[seed_affinity] 事务回滚,未做任何修改'))
raise
for m in messages:
self.stdout.write(m)
```
方案 A 更符合 seed 命令的运维场景部分失败可重跑方案 B 更符合"全有全无"的事务语义建议方案 A
**影响范围:**
- 运维诊断
- 数据一致性误判
---
### WR-008ParadiseUser.favorability 旧字段保留但无运行时拦截,存在双轨数据写入风险
**File:** `qy_lty/userapp/models.py:29`
**Issue:**
```python
favorability = models.IntegerField('好感度', default=0)
```
数据迁移 0006 把数据搬到 `UserDevice.favorability` `ParadiseUser.favorability` 仍然是普通字段可读可写但代码库内仍有引用风险
- `userapp/serializers.py` 可能仍序列化此字段未审查到具体文件 grep
- 管理后台 `userapp/admin.py` 可能仍在编辑界面暴露
- 老的 API 客户端可能仍在 PATCH 这个字段
- 团队成员可能在 P2 写新代码时仍然 `user.favorability += 1`
一旦双写存在数据双轨`ParadiseUser.favorability` vs `UserDevice.favorability` 不一致后续审计极痛苦
**Fix:**
分两步
第一步P1 收尾立即做把字段标 deprecated model override `__setattr__` property 拦截写
```python
# userapp/models.py
@property
def favorability(self):
import warnings
warnings.warn(
'ParadiseUser.favorability 已弃用,请使用 UserDevice.favorability。'
'此 property 仅返回主设备值,不应用于业务逻辑。',
DeprecationWarning, stacklevel=2,
)
primary = self.devices.filter(is_primary=True, is_active=True).first()
return primary.favorability if primary else 0
@favorability.setter
def favorability(self, value):
raise AttributeError(
'ParadiseUser.favorability 已弃用,写操作不允许。请操作 UserDevice.favorability。'
)
```
property 不能直接覆盖 model field——需要重命名 DB 字段为 `_legacy_favorability` 然后加一个迁移
```python
# 0007_deprecate_user_favorability.py
operations = [
migrations.RenameField(
model_name='paradiseuser',
old_name='favorability',
new_name='_legacy_favorability',
),
]
```
第二步下个版本彻底 `migrations.RemoveField`同时审计所有 `serializers.py` / `admin.py` / `views.py` 中对 `paradiseuser.favorability` 的引用
**影响范围:**
- 数据一致性
- 跨模块代码引用清理
**注意:** 由于该 property 形式会触发 N+1每次访问都查 devicesP2 后端的 serializer 应该改为直接展示 UserDevice 列表而不是 ParadiseUser.favorability在做 property 改造前先全文搜索 `favorability` 字段被哪里读写
```
grep -rn 'favorability' qy_lty/ --include='*.py'
```
并清单化每个调用点的修复计划
---
### WR-009AffinityLevel 区间允许重叠 / 留空隙,等级匹配结果不确定
**File:** `qy_lty/userapp/models.py:174-244`
**Issue:**
设计文档 §6.2 假定等级区间是 `[0,20], [21,40], [41,60], [61,80], [81,100]` 完整覆盖且不重叠 model 没有任何约束防止
- 重叠Lv2 `min_affinity=21, max_affinity=40`Lv3 `min_affinity=35, max_affinity=60` 好感度 38 同时匹配 Lv2 Lv3服务端取等级取决于查询排序
- 空隙Lv2 `[21,40]`Lv3 `[50,60]` 好感度 45 的设备等级查不出来缓存 `affinity_level` 字段无值
P2 / P3 服务层等级计算 `AffinityLevel.objects.filter(min_affinity__lte=v, max_affinity__gte=v).first()` 会静默取一个不报错
**Fix:**
PostgreSQL `EXCLUDE` constraint + `btree_gist` 扩展可以保证区间不重叠
```python
# AffinityLevel.Meta
constraints = [
ExclusionConstraint(
name='affinitylevel_no_overlap',
expressions=[
(NumRange('min_affinity', 'max_affinity', '[]'), '&&'),
],
),
]
```
但需要在迁移里 `CREATE EXTENSION btree_gist`部署成本上升
更简单的方案 `AffinityLevel.clean()` Python 校验 `seed_affinity` / management command `verify_affinity_levels` 检查完整覆盖
```python
def clean(self):
super().clean()
if self.min_affinity > self.max_affinity:
raise ValidationError({'max_affinity': '上限不能小于下限'})
# 与其他等级的重叠检查
overlaps = AffinityLevel.objects.exclude(pk=self.pk).filter(
Q(min_affinity__lte=self.max_affinity) &
Q(max_affinity__gte=self.min_affinity) &
Q(is_deleted=False)
)
if overlaps.exists():
raise ValidationError(
f'与等级 {", ".join(str(l) for l in overlaps)} 区间重叠'
)
```
并在 admin / serializer 显式 `full_clean()`
**影响范围:**
- 等级计算的正确性
- 缓存 `UserDevice.affinity_level` 字段的可信度
---
## Info
### IN-001AffinityRule 旧字段 points / daily_limit / is_active 缺乏 db_column 与显式 deprecation 路径
**File:** `qy_lty/userapp/models.py:148-160`
**Issue:**
注释说"下个版本删除"但没有
- TODO / FIXME 关联 ticket / version
- `RemovedInVNextWarning` 装饰
- 哪个版本会删的具体说明
后续开发者包括 LLM agent查这些字段时只能靠 docstring
**Fix:**
加显式注释
```python
points = models.IntegerField(
'积分(已弃用)', default=0,
help_text='[DEPRECATED v0.3 → v0.4 删除] 使用 min_change/max_change',
)
```
或者用 Python warnings model property 上拦截访问同时在 `docs/好感度系统-开发任务清单.md` 加一条"P1-收尾清理 deprecated 字段"。
---
### IN-002DEFAULT_RULES 和 DEFAULT_LEVELS 应抽到独立模块便于复用
**File:** `qy_lty/userapp/management/commands/seed_affinity.py:28-112`
**Issue:**
两个常量 dict 占了 80% 文件 P2 单元测试 / 集成测试也会需要这些 fixture"给我一个默认 chat 规则")。当前埋在 management command 文件里只能通过 `from userapp.management.commands.seed_affinity import DEFAULT_RULES` 导入路径丑陋且非常规
**Fix:**
```python
# qy_lty/userapp/affinity/defaults.py
DEFAULT_RULES = [...]
DEFAULT_LEVELS = [...]
DEFAULT_SETTING = {...}
# seed_affinity.py
from userapp.affinity.defaults import DEFAULT_RULES, DEFAULT_LEVELS
```
测试代码P2 service 文档生成都可以复用
---
### IN-003AffinityRule.daily_cap 与 AffinitySetting.daily_cap 同名易混淆
**File:**
- `qy_lty/userapp/models.py:124` (`AffinityRule.daily_cap`)
- `qy_lty/userapp/models.py:263` (`AffinitySetting.daily_cap`)
**Issue:**
P2 / P3 服务层代码会同时引用两者`if today_total >= rule.daily_cap` `if global_today_total >= setting.daily_cap` 容易拼错
**Fix:**
建议把 `AffinitySetting.daily_cap` 重命名为 `global_daily_cap` `daily_cap_global`让一眼区分需要一个 `RenameField` 迁移成本不高
---
### IN-004AffinityLog __str__ 在 self.id 为 None未保存时会显示 #None
**File:** `qy_lty/userapp/models.py:401-402`
**Issue:**
```python
def __str__(self):
return f"#{self.id} {self.rule_key or self.source} {self.before_value}->{self.after_value}"
```
`AffinityLog(...)` 但未 save `self.id` None调试输出形如 `#None chat 0->5`无功能影响但不优雅
**Fix:**
```python
def __str__(self):
return f"#{self.pk or 'new'} {self.rule_key or self.source} {self.before_value}->{self.after_value}"
```
---
### IN-005UserDevice.is_active 与 Device.is_active 命名冲突help_text 已警告但仍建议改名
**File:** `qy_lty/device_interaction/models.py:122-126`
**Issue:**
`Device.is_active` 50表示"设备已激活"`UserDevice.is_active`新增表示"绑定关系有效"。help_text 已经显式说" Device.is_active 不是同一概念"——这本身就是一个 code smell 信号
后续 `select_related('device')` `ud.device.is_active` vs `ud.is_active` 表达截然不同的语义极易拼错
**Fix:**
建议把 `UserDevice.is_active` 重命名为 `is_binding_active` `is_bound`
```python
is_bound = models.BooleanField(
'绑定有效', default=True,
help_text='软删除标记。解绑置为 false重绑时可读取历史值。',
)
```
需要新迁移 `RenameField` P1 尚未上线按用户描述"纯数据层工作"改名成本低如果已有数据写入必须按 CR-001 同步修改所有调用点
---
### IN-0060006 数据迁移 print 输出无前缀格式,迁移日志难以 grep
**File:** `qy_lty/userapp/migrations/0006_migrate_favorability_to_userdevice.py:60-64, 85`
**Issue:**
```python
print(
f"\n[P1-09] favorability 数据迁移完成:"
f"成功 {migrated_count},无设备跳过 {skipped_no_device}"
f"零值跳过 {skipped_zero}"
)
```
Django 推荐迁移内用 `schema_editor.connection.ops.executor.stdout` `apps.get_app_config('userapp').stdout`更标准的是 `print` 但加上 `[migration 0006_xxx]` 前缀当前以 `[P1-09]` 业务标识写死未来根据迁移文件名查找时不方便
**Fix:**
```python
print(f"\n[migration 0006_migrate_favorability] forward: 成功={migrated_count}, "
f"无设备={skipped_no_device}, 零值={skipped_zero}")
```
并把所有迁移内 print 改用统一格式
---
## 修复优先级建议
| 优先级 | 编号 | 阻塞 | 建议时机 |
|--------|------|------|----------|
| P0 | CR-001 | P2 软删 / 已存在功能回归 | **必须** P2 service 层落地前修复最迟本周 |
| P0 | CR-002 | P2 service 层稳定性 | **必须** P2 service 层落地前修复 |
| P0 | CR-003 | 重跑迁移 / 数据完整性 | 立即修复已有迁移+ 写运维 SOP "不可重跑此迁移" |
| P1 | WR-001 ~ WR-009 | 部分阻塞 P2/P3 | P2 第一周完成 |
| P2 | IN-001 ~ IN-006 | 不阻塞 | P2/P3 渐进改造 |
特别注意 CR-001 IN-005 是配对的——改名 + manager + 修所有调用点 + 更新 CLAUDE.md应作为同一个 commit 完成避免半成品状态
---
## Cross-Module 调用链分析deep 模式补充)
按用户在配置中明确要求做 cross-app FK cross-module 调用链审查补充如下
### Call Chain 1MAC 登录 → UserDevice 控制权解析
```
MAC 设备 →
userapp/views.py:120 UserDevice.objects.filter(device=device).order_by('-bound_at').first() →
签发 user-token →
device_interaction/auth.py 解析 token →
WS connect → group = device_{user_id}
```
**风险:** 全程未过滤 `is_active`详见 CR-001
### Call Chain 2好感度变更 → 日志 → 等级 → 奖励发放(未来 P2/P3
```
事件接收 →
AffinityRule 查询(按 rule_key
random.randint(min_change, max_change) →
UserAffinityDailyCounter 累加 + AffinitySetting.daily_cap 检查 →
UserDevice.favorability 写 →
AffinityLog 写event_id 幂等)→
AffinityLevel 区间匹配 →
UserLevelRewardGrant 写unique 防重复)→
WebSocket 推送
```
**潜在风险点:**
- AffinityRule.min_change > max_change → `random.randint`CR-002
- AffinityRule 不存在 `companion_time` 规则 → 陪伴时长事件被忽略WR-005
- AffinityLevel 区间重叠 → 等级匹配结果不确定WR-009
- UserLevelRewardGrant.device CASCADE → 设备删后奖励历史丢失WR-002
- AffinityLog.event_id 用 `''` 而非 NULL → 幂等失效边界 caseWR-004
### Cross-App FK 一致性检查
| FK | on_delete | 设计意图 | 实际表现 | 一致? |
|----|-----------|---------|---------|--------|
| AffinityLog.device | SET_NULL | 历史保留 | ✓ | ✓ |
| AffinityLog.rule | SET_NULL | 规则删后日志保留 | ✓ + rule_key 冗余 | ✓ |
| AffinityLog.user | CASCADE | 用户注销清理 | ✓ | ✓ |
| UserAffinityDailyCounter.device | CASCADE | 计数器随设备删 | ✓ | ✓(合理) |
| UserAffinityDailyCounter.rule | CASCADE | 规则删后计数器删 | ⚠ 与 AffinityLog.rule SET_NULL 不一致 | 部分一致 |
| UserLevelRewardGrant.device | CASCADE | ? | 设计意图未明确,造成歧义 | ✗ (WR-002) |
`UserAffinityDailyCounter.rule` 用 CASCADE 也值得讨论:一条 rule 被软删后(`is_deleted=True`),新事件不会再用,但旧的当天计数器仍在使用中。建议保持 CASCADE 但补充设计文档说明"软删 rule 时业务必须等当天计数器清零再硬删"。
---
_Reviewed: 2026-05-13T00:00:00Z_
_Reviewer: Claude (gsd-code-reviewer)_
_Depth: deep_
_Note: 本次审查不在 GSD .planning/phase-N 结构内,按用户要求保存到 docs/REVIEW-affinity-P1.md_