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>
39 KiB
phase, reviewed, depth, files_reviewed, files_reviewed_list, findings, status
| phase | reviewed | depth | files_reviewed | files_reviewed_list | findings | status | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| affinity-P1 | 2026-05-13T00:00:00Z | deep | 6 |
|
|
issues_found |
好感度系统 P1 数据层代码审查报告
审查时间: 2026-05-13 审查深度: deep(含 cross-app FK 与 cross-module 调用链分析) 审查范围: P1 阶段(P1-01 ~ P1-10)数据层产出物,共 6 个文件 状态: issues_found(3 critical / 9 warning / 6 info)
Summary
P1 阶段建模整体方向是合理的:把好感度从 ParadiseUser 下沉到 UserDevice 是符合"一人多设备 / 设备级人格"业务语义的正确选择;规则 / 等级 / 日志 / 计数器 / 奖励发放记录五张表的拆分清晰;冗余 rule_key 字段、event_id 部分唯一索引、reward_snapshot JSON 快照等都是经验性的好实践。
但仍存在三类共 18 个需要在 P2 服务层动工之前修复的问题:
- 完整性约束严重缺失(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,日上限会永远命中或永远不命中,是线上爆炸级风险。 - 既有"换绑挤掉旧绑定"语义被
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}。这是直接破坏运行中功能的语义回归。 - 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-001:UserDevice 既有"最新绑定者"取数逻辑未过滤 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:
# 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 强制:
# 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-002:AffinityRule / 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 级保护:
min_change > max_change→random.randint抛ValueError,整个 apply 流程崩溃cooldown_seconds < 0→ 冷却计算last_trigger + timedelta(seconds=cooldown)得到过去时刻,永久解锁single_cap <= 0→ 任何变化值都被钳为 0daily_cap <= 0→ 第一次写入就触发上限AffinitySetting.decay_min_decay > decay_max_decay→ 衰减任务崩AffinitySetting.max_affinity < initial_affinity→ 新设备初始好感度已超上限AffinityLevel.min_affinity > max_affinity→ 该等级永远不触发- 多个
AffinityLevel的区间可以重叠(如 Lv2 [21,40] 与 Lv3 [35,60]),服务端等级匹配结果取决于查询顺序,不确定 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:
# 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-003:0006 数据迁移用 target.favorability == 10 做幂等条件,存在数据覆盖与回滚失效双重风险
File: qy_lty/userapp/migrations/0006_migrate_favorability_to_userdevice.py:54-58, 67-85
Issue: forward 函数的幂等条件:
if target.favorability == 10: # 行 55
target.favorability = favorability
target.save(update_fields=['favorability'])
migrated_count += 1
backward 函数的回滚条件:
if primary and primary.favorability != 10: # 行 80
user.favorability = primary.favorability
user.save(update_fields=['favorability'])
10 是 UserDevice.favorability 字段的 model 默认值,也是 AffinitySetting.initial_affinity 的默认值,意味着:
- 未来某用户合法地把好感度衰减到 10(衰减任务跑了一段时间后),如果运维误执行
migrate --fake后再unfake或重做某次回滚 / 重跑,forward 会再次把 ParadiseUser.favorability 旧值(很可能是迁移时的快照)写回,覆盖正常衰减后的当前值,造成业务数据错乱 - 管理员手工把好感度调整为 10,下一次重跑迁移同样会被覆盖
- backward 在系统稳定运行一段时间后执行,由于 forward 早已完成,所有非 10 的设备会被回写到
ParadiseUser.favorability——但ParadiseUser.favorability此时是过期数据(业务代码已经全部走UserDevice),回写没有意义;同时所有当前正好为 10 的设备会被跳过回写,导致迁移前那批好感度 = 10 的用户永久丢失原 favorability 值(如果他们在 forward 时本来有非零 ParadiseUser.favorability 但被跳过) - 用户提到迁移已经在 makemigrations 后第二次启动时跑过一次,输出"9 个零值跳过 + 1 个无设备跳过 + 0 成功"——这意味着实际成功迁移条数为 0,但
MigrationRecorder已记录迁移完成。如果以后再有用户从老库恢复数据(FK fixture / dumpdata),无法再次自动迁移
Fix:
方案 A(推荐,引入幂等标记):用 AffinityLog 或 metadata 标记已迁移:
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 False),forward 写入时设为 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-001:AffinitySetting.save() 单例保证在并发下仍可能制造重复行
File: qy_lty/userapp/models.py:303-308
Issue:
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)
在多 worker(gunicorn / daphne 多进程)或并发 admin 操作下,两个进程的 AffinitySetting.objects.exists() 调用可以同时返回 False(首次部署 / 表为空时),随后两个 INSERT 都成功,得到两条记录。即便表不为空,AffinitySetting.objects.first() 读到的两个 pk 可能不同(理论上单例表不存在这种情况,但代码不应假设),随后两个 UPDATE 都成功——总体上单例保证是 best-effort 的,不是强约束。
Fix:
- 把
get_solo()改为pk=1硬编码 +update_or_create:
@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)
- 同时加 DB 级 CHECK 约束防御:
constraints = [
models.CheckConstraint(check=Q(pk=1), name='affinitysetting_singleton'),
]
虽然 CHECK 约束在 PostgreSQL 下不能跨行验证(不能强制"全表只有 1 行"),但 pk=1 约束可以阻止任何非 1 的主键,配合应用层 force pk=1 就能形成事实单例。
影响范围:
- 影响低(首次部署窗口期),但配置型数据出现重复行后续排查极痛苦
WR-002:UserAffinityDailyCounter / 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:
# 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-003:AffinityLog 索引设计偏重,写入开销可能放大 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_idpartial 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 等只增字段非常高效):
from django.contrib.postgres.indexes import BrinIndex
indexes = [
models.Index(fields=['device', '-created_at']),
BrinIndex(fields=['created_at']),
]
影响范围:
- 写入吞吐
- 表空间占用
WR-004:unique_affinity_event_id 部分唯一索引语义有歧义,空字符串作为"无 event_id"标记不安全
File: qy_lty/userapp/models.py:393-399
Issue:
constraints = [
UniqueConstraint(
fields=['event_id'],
condition=Q(event_id__gt=''),
name='unique_affinity_event_id',
),
]
PostgreSQL 下这会生成 CREATE UNIQUE INDEX ... WHERE event_id > '',是合法的 partial index。但有两个问题:
event_id__gt=''语义脆弱:依赖event_id是字符串、依赖''是 "无值"标记。如果未来某客户端 bug 传了' '(空格)或'null',会被当作有效 event_id 进入唯一约束。- 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:
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-005:seed_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:
# 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-006:AffinityLevel.description 在 seed 中未设置,依赖 model blank=True 隐式 '';未来字段改为 required 会一次性报错
File:
qy_lty/userapp/management/commands/seed_affinity.py:81-112qy_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)或加 unique,seed 会一次性失败。
Fix: 显式补齐:
DEFAULT_LEVELS = [
{
'level': 1, 'name': '初识',
'description': '初次相识阶段,了解彼此的基础阶段', # 显式
# ...
},
...
]
模型侧也建议补 default='':
description = models.TextField('等级描述', blank=True, default='')
AffinityRule.description 同样问题。
影响范围:
- 健壮性
- 未来 schema 演进
WR-007:seed_affinity @transaction.atomic 包整个 handle,force 模式下部分失败会回滚所有已处理项
File: qy_lty/userapp/management/commands/seed_affinity.py:124-136
Issue:
@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(推荐,每条独立事务):
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 改为推迟输出):
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-008:ParadiseUser.favorability 旧字段保留但无运行时拦截,存在双轨数据写入风险
File: qy_lty/userapp/models.py:29
Issue:
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 拦截写:
# 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 然后加一个迁移:
# 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(每次访问都查 devices),P2 后端的 serializer 应该改为直接展示 UserDevice 列表而不是 ParadiseUser.favorability。在做 property 改造前,先全文搜索 favorability 字段被哪里读写:
grep -rn 'favorability' qy_lty/ --include='*.py'
并清单化每个调用点的修复计划。
WR-009:AffinityLevel 区间允许重叠 / 留空隙,等级匹配结果不确定
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、Lv3min_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 扩展可以保证区间不重叠:
# 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 检查完整覆盖:
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-001:AffinityRule 旧字段 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: 加显式注释:
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-002:DEFAULT_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:
# 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-003:AffinityRule.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-004:AffinityLog str 在 self.id 为 None(未保存)时会显示 #None
File: qy_lty/userapp/models.py:401-402
Issue:
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:
def __str__(self):
return f"#{self.pk or 'new'} {self.rule_key or self.source} {self.before_value}->{self.after_value}"
IN-005:UserDevice.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:
is_bound = models.BooleanField(
'绑定有效', default=True,
help_text='软删除标记。解绑置为 false,重绑时可读取历史值。',
)
需要新迁移 RenameField,但 P1 尚未上线(按用户描述"纯数据层工作"),改名成本低。如果已有数据写入,必须按 CR-001 同步修改所有调用点。
IN-006:0006 数据迁移 print 输出无前缀格式,迁移日志难以 grep
File: qy_lty/userapp/migrations/0006_migrate_favorability_to_userdevice.py:60-64, 85
Issue:
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:
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 1:MAC 登录 → 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 → 幂等失效边界 case(WR-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