# Code Review Report — QCDistributionRegisterInfo 重复登记防御修复

- **Commit**: `a3f4e4f` 修复 QCDistributionRegisterInfo 重复登记问题
- **分支**: master3
- **评审级别**: L3（核心业务 / 数据一致性 / 无 DB 约束兜底）
- **评审者**: 独立上下文 subagent（不知任何实现过程）
- **评审日期**: 2026-04-13
- **硬约束**（来自用户）：不加唯一索引、不引入 SERIALIZABLE 事务

---

## 1. 总评分与结论

**总分：6.2 / 10 — 不通过，必须修复后再评。**

本次修复整体方向正确（业务层双层查重 + 收口合并规则 + ImportLabs 去重 + 日志告警），架构思路站得住脚。但**存在一处 Critical 级回归**：`QCDistributionRegisterInfoViewModel.ToEntityByLabCode` 新加的 `ProjectId` 过滤条件，会让 `ConfigrmFee` / `ConfigrmEMS` / `switchNextOne` 三个后台收费/寄送 ajax 动作在当前前端实现下 100% 静默失败（详见第 6/10 节）。另外日志告警命名有效性、`MergeRegisterInfo` 对 string 字段的对称性、以及 `ImportLabs` 的二次脏数据风险各有中等问题。修掉 Critical 后复评有望到 9.0+。

---

## 2. 各维度打分表

| 维度 | 权重 | 分数 | 主要扣分点（带证据） |
|------|------|------|------|
| 正确性 | 30% | 5.0 | **Critical**：`QCDistributionRegisterInfoViewModel.cs:73` 加的 `p.ProjectId == regInfoivewModel.ProjectId` 对当前前端是致命回归（见 §6）；`MergeRegisterInfo` 对 string 的"仅非 null 才覆盖"无法区分"要清空"和"不想改"（见 §7）；`ImportLabs` 的 `GroupBy.Select(g => g.First())` 在两条源行 IsCharged 不一致时会静默丢掉一条（见 §5） |
| 规范 | 20% | 7.5 | `LogHelper.Info("[WARN] ...")` 用文本前缀伪装告警级别，不符合 log level 语义（见 §9）；`MergeRegisterInfo` 作为静态方法写在 `QCService` 里职责混合，命名没有文档注释标注"不对称"规则；`QCDistributionInfoViewModel.cs:144-156` 的查重分支与 `QCService.SaveQcDistributionRegister` 的查重分支逻辑**不一致**（第一道防线只同步两个字段，第二道防线走 MergeRegisterInfo 合并全部字段）——两个"双保险"实际规则不同（见 §5） |
| 测试 | 20% | 4.0 | 仓库无单测工程（已知）。本次 Critical 路径若走一次真实手测即可暴露（开一次"收费确认"对话框，填 LabCode 点"确认"），但 commit 未附任何手测剧本说明、也没 changelog。该分数是无单测 + 无手测证据联合结果 |
| 安全 | 15% | 8.0 | 无 SQL 注入/越权/信息泄露新增面。残余并发窗口是已知、已文档化、明确不修复。扣分点在于 `[WARN]` 日志监控在 NLog 默认级别过滤下基本不会被运维告警系统发现，使"靠日志监控"这一缓解手段形同虚设 |
| 性能/可维护性 | 15% | 7.5 | 每次 `SaveLabList` 在循环内做一次 `FirstOrDefault` 查重（N+1 式，N = LabList.Count），7000 行量级可接受但不优（见 §4）；`MergeRegisterInfo` 共 22 个 `if`，新增字段时容易漏改；`CLAUDE.md` 契约章节写得好，给未来维护者留了足够上下文，加分 |
| **加权合计** | — | **6.23** | — |

**分数门槛**：L3 需 ≥ 9.0。当前 6.23，**不通过**。

---

## 3. 边界场景—代码覆盖映射表

仓库无单测工程，下列"测试证据"一列均为**手测剧本**或**对应代码段保证**。带 ❌ 的场景即为"防线未覆盖"。

| # | 边界场景 | 由哪段代码兜底 | 证据/手测剧本 | 状态 |
|---|---|---|---|---|
| 1 | 同一分发 + 同 Lab + 同 Project，先新建再新建（前端漏查重） | `QCService.cs:1037-1054` 查重命中 → Merge → Update；`QCDistributionInfoViewModel.cs:146-156` 第一道防线 | 手测：SaveLabList 把同一行勾选两次连续保存，DB 应仍 1 行 | ✅ |
| 2 | 同一分发 + 同 Lab + 不同 Project（合法场景） | `QCService.cs:1040` 查重条件包含 ProjectId，不会误命中；`QCDistributionInfoViewModel.cs:149-150` 同上 | 手测：一个实验室勾选 ProjectId=1 和 ProjectId=2 两个项目保存 | ✅ |
| 3 | 历史脏行（DB 已有两条重复）再次 Save（Id=0） | `QCService.cs:1037` 的 `FirstOrDefault` 只会合并到其中一条，另一条仍留在 DB | 手测：人为插入重复行再触发保存，DB 仍有两条 | ⚠️ 无兜底，靠上一轮手工 SQL 清理 |
| 4 | `ConfigrmFee` 前端提交不带 ProjectId（当前 cshtml 真实情况） | `QCDistributionRegisterInfoViewModel.cs:73` 查询变成 `ProjectId == 0` 永远命中不到，`entity == null` 直接 return（BackstageController.cs:218-219） | 手测：打开 QCDistributionLabs 页面→点"收费确认"→输入 LabCode→点确认。**应失败/静默无变化** | ❌ **Critical 未覆盖** |
| 4a | `ConfigrmEMS` 同上 | 同 §4 | 手测：点"标本寄出确认"→填 EMSNo→确认 | ❌ **Critical 未覆盖** |
| 4b | `switchNextOne` 同上 | 同 §4；`BackstageController.cs:301` 拿不到 entity，无法取下一条 | 手测：收费对话框里点"下一个"按钮 | ❌ **Critical 未覆盖** |
| 5 | `ImportLabs` 源分发里存在 (LabId,ProjectId) 重复 | `BackstageController.cs:283-287` `GroupBy(...).Select(g => g.First())` | 手测：把源分发 41 复制到目标分发 99，检查目标表记录数 | ⚠️ 信息丢失：若两条源行 IsCharged/AnswerJSON 不同，`.First()` 静默丢弃后者 |
| 6 | `ImportLabs` oriDistId == TargetDistId | `BackstageController.cs:266-269` 前置守卫直接 return | 手测：在 UI 里选择相同分发作为源和目标 | ✅ |
| 7 | `ImportLabs` TargetDistId 不存在 | `BackstageController.cs:270-274` 前置守卫 | 手测：构造 TargetDistId=9999 的请求 | ✅ |
| 8 | 并发：两个管理员同时 Save 同一 (dist,lab,project) | **无兜底**，两 DbContext 查重都返回空 → 两个 Insert 都跑 → 重复行 | CLAUDE.md:131 已明确标"残余风险" | ⚠️ 已知接受 |
| 9 | `saveAnswerInfo`(UserUI) 入参 Id=0（理论不应发生，但前端有可能 round-trip 时丢 Id） | `QCService.cs:1037-1054` 收口查重 + MergeRegisterInfo | 手测：抓包把 Id 改为 0 再提交 | ⚠️ 部分覆盖，但 MergeRegisterInfo 的 string 规则会保留既有脏答案 |
| 10 | `Id=0` + `IsCharged=false` 想"反收费" | `QCService.cs:1072` `if (source.IsCharged) target.IsCharged = true;` **故意不支持 false→true**，静默忽略 | CLAUDE.md:123 已文档化（要求走 Id>0 路径） | ⚠️ 有设计意图，但无代码层面拒绝（无抛异常/日志） |
| 11 | `Id=0` + `ChargeTime=null` 想"清空收费时间" | 同上，`ChargeTime.HasValue` 为 false 时保留既有值 | CLAUDE.md 未明确列出"清空场景需走 Id>0" | ⚠️ 同 §10 |
| 12 | `SaveQcDistributionRegister` 命中已有行时，`qcDistributionRegister.Id = existing.Id` 回写 | `QCService.cs:1046` 回写，防止调用方把"新" entity 当 insert 成功后又用 Id=0 重复 Save | 代码静态审查 OK | ✅ |
| 13 | `QCDistributionInfoViewModel.ToEntity` 第一道防线命中时，传入的 `registerInfo` 对象被丢弃，只同步 `IsCharged`+`ModifyTime` | `QCDistributionInfoViewModel.cs:152-155` | 代码审查：**其他字段（Remark/SampleNo 等）都不同步**——如果 ViewModel LabList 里有 Remark 更新，这里会被静默丢失 | ⚠️ 与第二道防线规则不一致（见 §5） |

**硬指标**：14 个场景里 **3 个 Critical 未覆盖（4 / 4a / 4b）、5 个中等问题、6 个通过**。按 L3 规则：**映射表存在 ❌，不得给 9.0+。**

---

## 4. 时空复杂度评估

数据规模参考：`QCDistributionRegisterInfoes` ≈ 7000 行，单次 `SaveLabList` 的 `LabList.Count` ≈ 几十到几百（视分页），`ImportLabs` 一次性 ≈ 几百到 2000。

| 代码段 | 时间复杂度 | 空间复杂度 | 说明 |
|---|---|---|---|
| `QCService.SaveQcDistributionRegister` 新增 `FirstOrDefault` 查重 (QCService.cs:1037) | EF 翻译为单条 `SELECT TOP 1 WHERE dist=? AND lab=? AND proj=?`，若 `(dist,lab)` 或 `(dist)` 有非聚集索引则 O(log N)，否则 table scan O(N)。N=7000 时即使全表扫也 <10ms，可接受 | O(1) | **风险**：若未来 N → 百万级且无索引会成瓶颈。建议在 CLAUDE.md 契约章节里补一句"如果 DBA 同意加非聚集索引（不是唯一索引），推荐 `(QCDistributionId, LabId, ProjectId)` 以支撑此查重" |
| `MergeRegisterInfo` (QCService.cs:1061-1084) | O(1)（固定 22 个字段） | O(1) | 无问题 |
| `BackstageController.ImportLabs` 的 `.ToList().GroupBy(...).Select(g => g.First()).ToList()` (BackstageController.cs:282-286) | 物化到内存后的 GroupBy：O(M)，M = 源分发行数。`.ToList()` → `GroupBy` → `Select` → `.ToList()` 意味着**两次物化**，内存占用 2×M | O(M) | M≈几千时 MB 级，可接受。风格上可改为直接 `Distinct(comparer)` 或用 `HashSet<(int,int)>` 手工去重，少一次物化，但不是必须 |
| `QCDistributionInfoViewModel.ToEntity` 循环内查重 (QCDistributionInfoViewModel.cs:148-151) | 循环体内 `FirstOrDefault` → N 次 DB round-trip，每次 O(log K)（K=7000）。总体 O(N log K) | O(1) per iter | **次优**：LabList 常见几十到几百行，即 200 次 DB 查询，每次 5-20ms → 单次请求 1-4 秒。不是 Critical，但用户体验会肉眼可感。**建议**：循环前一次性 `.Where(p => p.QCDistributionId == viewModel.Id).ToList()` 拉到内存（≈几百行），然后在内存里 `FirstOrDefault`，复杂度降到 O(1 次 DB + N log K 内存) |

---

## 5. 并发/竞态风险点

| 场景 | 是否被本次修复覆盖 | 判断 |
|---|---|---|
| 同一用户快速双击"保存" | ❌ 不覆盖（两个请求仍会各自走一遍业务层查重，但两次 DbContext 都可能在读后写前完成） | 低概率。前端可加按钮防抖作为缓解，但 commit 未提及 |
| 两个管理员并发编辑同一分发 | ❌ 不覆盖 | CLAUDE.md:131 已明确写明残余风险，是**可接受**的 |
| `SaveLabList` 循环内部非事务地逐行 Save | ❌ 不覆盖 | 任一行失败会留下部分状态，但这是原有问题，本次修复未引入恶化 |
| `ImportLabs` 先 Delete 再 Add 非事务 | ❌ 不覆盖 | 同上，原有问题。`qcDistInfo.QCDistributionRegisters.Add(newEntityInfo)` 通过导航集合写入，会被后续 `SaveQcDistribution` 的 SaveChanges 批量提交，比逐条稍好 |
| `QCService.SaveQcDistributionRegister` 内的 "查重 → Update" 无悲观锁/乐观锁 | ❌ 不覆盖 | 无 RowVersion / Timestamp 字段，无法做乐观并发。残余风险和"两个管理员并发"同源 |

**可接受性判断**：本次 commit 作为无 DDL、无 TransactionScope 的"业务层尽力而为"方案，并发残余风险已在 CLAUDE.md 明确告知并被用户接受。**此项不扣分**（硬约束决定的）。唯一建议是 §9 的日志监控可行性，让残余风险发生时能及时发现。

---

## 6. 集成一致性检查结果

### 6.1 双层查重规则对齐检查

| 接口/动作 | 提供方 | 消费方 | 查重逻辑 | 合并规则 | 一致性 |
|---|---|---|---|---|---|
| SaveLabList 第一道防线 | `QCDistributionInfoViewModel.ToEntity` (cs:148-156) | BackstageController.SaveLabList | 命中则**只同步 2 个字段**：`IsCharged`、`ModifyTime`，然后调 `SaveQcDistributionRegister(existing)`（此时 `existing.Id>0` → 走 Update 分支，不会再走 Merge） | — | ❌ **与第二道防线不对齐** |
| SaveQcDistributionRegister 收口防线 | `QCService.SaveQcDistributionRegister` (cs:1037-1053) | 所有 Id==0 的调用方 | 命中则用 `MergeRegisterInfo` 合并**全部 22 个字段** | 不对称合并（string 非 null / bool 只升） | — |

**问题**：第一道防线只同步 IsCharged/ModifyTime，而 `registerInfo` 里还有 `SampleNo`/`EMSNo`/`PacketContent` 等字段（在 `QCDistributionInfoViewModel.cs:140` 之前由 `ToEntity(x)` 从 ViewModel 拷贝过）。命中已有行时这些字段**会被静默丢弃**。如果用户在 SaveLabList 页面编辑了 Remark 然后保存，Remark 更新会被丢掉。

**证据**：`sbcLabSystem/Models/Backstage/QCDistributionInfoViewModel.cs:140-156`

**建议**：第一道防线应当和第二道防线统一走 MergeRegisterInfo（把它从 QCService 里 public 化或独立到 util），或者至少显式同步同一组字段。当前状态是"两道防线规则分叉"，CLAUDE.md 声称"双保险"但实际两保的不是同一件事。

### 6.2 `ToEntityByLabCode` 新增 ProjectId 过滤 —— 所有调用方的 ProjectId 传递链路检查

| 调用方 | 入口 | 前端提交 payload 里是否一定有 ProjectId | 结论 |
|---|---|---|---|
| `BackstageController.ConfigrmFee` (cs:217) | `QCDistributionLabs.cshtml:191` `confirmFee` 函数 | **否**。`openFeeWindow` 初始化字面量 `{"LabCode":"","LetterNo":"","ChargeRemark":"","QCDistributionId":"{0}","IsCharged":false}` **不含 ProjectId**。对话框 HTML (`cshtml:385-406`) **无 ProjectId 输入控件**。用户无法输入 ProjectId。即使 `CurrentQCDistRegisterInfo` 在页面初始化时从后端 `QCDistributionPageViewModel.CurrentQCDistRegisterInfo = new QCDistributionRegisterInfoViewModel()` 拿到 `ProjectId=0`，提交时也是 0 | ❌ **Critical 回归** |
| `BackstageController.ConfigrmEMS` (cs:241) | `cshtml:227` `confirmEMS` 函数 | 同上 | ❌ **Critical 回归** |
| `BackstageController.switchNextOne` (cs:301) | `cshtml:236` `switchNextOne` 函数 | 同上，依赖 `ToEntityByLabCode` 先拿到 entity 再找下一条 | ❌ **Critical 回归** |

**连锁反应**：
1. 前端 `confirmFee`：`ko.mapping.toJS` 序列化出 `{LabCode, LetterNo, ChargeRemark, QCDistributionId, IsCharged, InputElement, ProjectId:0, ...}`
2. 后端 `ConfigrmFee` 收到 `regInfoivewModel.ProjectId == 0`
3. `ToEntityByLabCode` 查询 `WHERE QCDistributionId=X AND LabId=Y AND ProjectId=0`，DB 里 ProjectId 最小是 1 → `entity == null`
4. `BackstageController.cs:218-219` 直接 `return Json(regInfoivewModel)` → 前端拿到对象后 `ko.mapping.fromJS(json, {}, viewModel.CurrentQCDistRegisterInfo)` 继续走，UI 可能弹 "收费成功" 或看似正常，**但 DB 没任何变化**
5. 原有 bug（QC269 两条重复行）本来就是靠 `ConfigrmFee` 改 IsCharged 时命中了"其中一条"来凑合工作的。修复后变成**两条都命不中**，比 bug 本身更糟

**必须修复路径**（三选一）：
- **(A) 前端补 ProjectId**：在 `cshtml:91` 和 `cshtml:108` 的 JSON 字面量里加 `"ProjectId":0` 不够（只把 0 显式化）；需要在对话框打开时或提交前从 LabList 找到 LabCode 对应的那一行拿 ProjectId 塞进去。**但问题**：一个 LabCode 可能在多个 ProjectId 都有记录，对话框里没给用户选，无法唯一确定
- **(B) 改回 ToEntityByLabCode 的查询条件**：保持原先的 `(QCDistributionId, LabId)` 匹配，不加 ProjectId；改动放到别的地方（比如对话框里加 ProjectId 下拉框）
- **(C) 最兼容方案**：在 `ToEntityByLabCode` 里做降级—— `if (regInfoivewModel.ProjectId > 0)` 才加 ProjectId 条件，否则退化为原行为（可能命中多条中的一条，但不会全部失效）。配合 CLAUDE.md 契约章节补一句"前端对话框场景下允许 ProjectId=0，此时退化为只按 LabId 匹配"

**我建议方案 (C) 作为最小代价回滚 + (A) 作为根治长期方向**。不修复这个 Critical，本次 commit 不应合入。

### 6.3 `ImportLabs` GroupBy 的信息丢失评估

`BackstageController.cs:282-286`:

```csharp
.GroupBy(x => new { x.LabId, x.ProjectId })
.Select(g => g.First())
```

**分析**：源分发若存在脏数据（两条 (LabId, ProjectId) 重复），`.First()` 只取第一条。被丢弃那条的 `IsCharged`/`AnswerJSON`/`Remark`/`LetterNo` 等状态全部丢失，目标分发只能拿到第一条的快照。

**是否影响业务**：ImportLabs 的功能是"把已有分发的实验室列表复制到新分发作为起点"。新分发是空的，源里的 IsCharged / AnswerJSON 从业务上讲**本来也不应**带到新分发（新一轮质评应该从 "未收费 / 未答题" 开始）。看 `cs:287-294` 的 `newEntityInfo` 构造：

```csharp
newEntityInfo.QCDistributionId = TargetDistId;
newEntityInfo.Id = 0;
newEntityInfo.LabId = x.LabId;
newEntityInfo.ProjectId = x.ProjectId;
newEntityInfo.ModifyTime = DateTime.Now;
```

**只复制了 LabId/ProjectId 两个维度标识**，根本不带 IsCharged/AnswerJSON 过来。所以 `.First()` 丢弃的信息**本来就会被丢弃**，**GroupBy 的信息丢失不影响业务**。✅

### 6.4 一致性汇总

| 接口 | 提供方 | 消费方 | 状态 | 偏差说明 |
|---|---|---|---|---|
| SaveLabList 双层查重规则 | ToEntity + SaveQcDistributionRegister | BackstageController.SaveLabList | ❌ | 第一道防线只同步 2 字段、第二道防线合并 22 字段 |
| `ToEntityByLabCode` ProjectId 过滤 | QCDistributionRegisterInfoViewModel | ConfigrmFee / ConfigrmEMS / switchNextOne | ❌ | 前端提交不含 ProjectId，3 个动作全部回归 |
| ImportLabs 去重 | BackstageController.ImportLabs | 自身 | ✅ | 信息丢失业务可接受 |
| SaveQcDistributionRegister 收口 | QCService | UserUIController.saveAnswerInfo / BackstageController.\* / QCDistributionInfoViewModel.ToEntity | ⚠️ | MergeRegisterInfo 的 string 规则下，脏 Id=0 提交不能清空字段 |
| ImportLabs 通过导航集合 Add 绕开 SaveQcDistributionRegister | BackstageController.ImportLabs | DbContext | ⚠️ | 已知绕开路径，靠自己 GroupBy 兜底，CLAUDE.md 已文档化 |

---

## 7. `MergeRegisterInfo` 不对称合并规则专项审查

### 7.1 逐字段审查

| 字段 | 类型 | 规则 | 正确性评价 |
|---|---|---|---|
| `LetterNo` | string | `source != null` 才覆盖 | ⚠️ 无法区分"空字符串=想清空"和"null=不想动"。目前 ToEntity 里空字符串被赋值为 `""` 而非 null（如 `entity.EMSNo = viewModel.EMSNo == null ? "" : viewModel.EMSNo.Trim();`，见 `QCDistributionRegisterInfoViewModel.cs:632`），所以 `source.LetterNo != null` 常常为真 → **实际上会覆盖**（可能会误覆盖成空串）。和"不会被意外 null 清空"的设计意图是矛盾的 |
| `ChargeRemark` | string | 同上 | 同上 |
| `SampleNo` | string | 同上 | 同上 |
| `EMSNo` | string | 同上 | 同上。特别注意：`ToEntity` 里 `entity.EMSNo = ""` 的路径会让 merge 误把已有 EMSNo 清空为 `""` |
| `PacketContent` / `Remark` / `ModifyUser` / `AnswerJSON` / `SubmitUserNo` / `Score_Detail` | string | 同上 | 同上 |
| `ChargeTime` / `SendEMSTime` / `SubmitTime` / `FirstTimeSubmitTime` / `LastPageModifyTime` | `DateTime?` | `HasValue` 才覆盖 | ✅ 可区分 null 和有值，正确 |
| `ModifyTime` | `DateTime`（非 nullable） | `!= default(DateTime)` 才覆盖 | ✅ 所有调用方都会 set `DateTime.Now`，实际上总会覆盖。OK |
| `IsCharged` / `IsSendEMS` / `IsSubmit` / `IsModified` / `IsEnding` | bool | `if (source.*) target.* = true` | ✅ 符合设计意图"防止脏 Save 把 true 清 false"，但反向意愿无法表达（CLAUDE.md:121-124 已文档化） |
| `QCDistributionId` / `LabId` / `ProjectId` | int | 不动 | ✅ 业务唯一键，正确 |

### 7.2 具体场景

**反例 — 证明 string 规则有 bug**：

> 管理员在一个已收费的 (dist=41, lab=5, proj=1) 行上（LetterNo="A1001", ChargeTime=2026-03-01, IsCharged=1）通过 `saveAnswerInfo`（UserUI 路径）提交了一份答题。UserUIController.cs:259 走 `QCDistributionRegisterInfoViewModel.ToEntity(viewModel)`。该 ToEntity 方法 (`cs:608-637`) 先用 `Id == viewModel.Id` 查，拿不到（比如 viewModel.Id 被前端 round-trip 成 0）就 `new QCDistributionRegisterInfo()` 配合 `ClassValueCopier.Mapper`。Mapper 会把 viewModel 的所有字段拷贝过去，包括 `LetterNo=""`（用户 answer 页面不填收费信息，默认值是空串）。然后落到 `SaveQcDistributionRegister` → 命中已有行 → `MergeRegisterInfo` → `source.LetterNo = ""` 不为 null → **target.LetterNo 被覆盖为空串，已收费的 A1001 编号丢失**。`ChargeTime` 因为是 `DateTime?` 且 viewModel 没 set 过保持 null，不会被清；`IsCharged` 因为 bool 规则不会被清。但 LetterNo 确实会丢。

**这与注释"防止脏 Save 把 IsCharged=1 意外清零"的设计意图部分达成，但 string 字段的保护是**半吊子的**。

**正例 — 证明 bool 规则对**：

> 同一场景下 `source.IsCharged = false`（新提交答题默认），`target.IsCharged = true`（已收费），`if (source.IsCharged)` 为 false → target 保持 true。✅

### 7.3 特别关注点（用户提出）

> 一个已有 `IsCharged=1` 的行被 `saveAnswerInfo(Id=0)` 路径命中合并后，既有的 `LetterNo` / `ChargeTime` / `ChargeRemark` 会不会被意外清空？

- `IsCharged`：✅ 不会被清（bool 规则保护）
- `ChargeTime`：✅ 不会被清（`DateTime?` + `HasValue` 规则，且 saveAnswerInfo 不 set ChargeTime）
- `LetterNo`：❌ **会被清空为 ""**（见上面反例，`ClassValueCopier.Mapper` 会把 viewModel 的空 LetterNo 写入 source）
- `ChargeRemark`：❌ **同上会被清空为 ""**

**结论**：`MergeRegisterInfo` 的 string 保护规则假设前提是 "source.XxxField 为 null 代表调用方不关心"，但仓库里的实际 ToEntity / ClassValueCopier 路径**不会产生 null**，只会产生 `""`。**保护规则在真实调用链下失效。**

### 7.4 修复建议

- 把 string 字段的"非 null"改成"非 null 且非 """ 才覆盖（即 `!string.IsNullOrEmpty(source.XxxField)`），并在 CLAUDE.md 契约里明确"要清空 string 字段必须走 Id>0 路径"。
- 或者更稳的做法：为 `saveAnswerInfo` 这条路径在 merge 时**显式白名单**只合并答题相关字段（AnswerJSON/SubmitTime/ModifyTime/IsSubmit），不要碰收费相关字段。但这需要调用方传递"意图"，改动更大。

---

## 8. 未被覆盖的路径盘点

搜索结果（grep `_qcDistributionRegisters.(Insert|Add)|QCDistributionRegisters.Add|QCDistributionRegisterInfoes.Add`）：

| 文件 | 行号 | 是否绕开 `SaveQcDistributionRegister` | 风险 |
|---|---|---|---|
| `sbcLabSystem.Service/QC/QCService.cs:1054` | `_qcDistributionRegisters.Insert(qcDistributionRegister)` | **否**，就是 SaveQcDistributionRegister 内部 | N/A |
| `sbcLabSystem/Controllers/BackstageController.cs:294` | `qcDistInfo.QCDistributionRegisters.Add(newEntityInfo)` | **是**（已知 ImportLabs 路径） | 已靠 GroupBy 兜底 + 目标表先清空 |
| `sbcLabSystem.Import/Form1.cs:36-60` | `current_db.QCDistributionRegisterInfoes.Add(regInfo)` + `current_db.SaveChanges()` | **是**（一次性数据迁移工具，winform） | **不影响线上**。这是一个旧版 MySQL→SQL Server 迁移 button，历史遗留工具，已完成使命，运行时不需要管 |

**答：绕开路径只有 2 处**：(1) 已知的 ImportLabs，已处理；(2) sbcLabSystem.Import.Form1 一次性迁移工具，不影响线上。搜索结果完整。✅

**搜索命令**：

```
Grep pattern: "_qcDistributionRegisters\.(Insert|Add)|QCDistributionRegister\.Add|\.QCDistributionRegisters\.Add"
```

---

## 9. 日志告警机制评估

`QCService.cs:1043`:
```csharp
LogHelper.Info(string.Format("[WARN] SaveQcDistributionRegister 命中已有行 ..."));
```

**问题 1 — Log level 语义错配**：
- `LogHelper.Info` 底层打的是 NLog **Info** 级别
- NLog 的告警规则一般只对 `WARN`/`ERROR`/`FATAL` 级别做邮件/短信/IM 转发
- `[WARN]` 文本前缀只能帮助人眼过滤日志文件，无法被日志监控系统识别
- 查 `LogHelper` 是否有 `Warn` 方法：grep 结果里没有找到 `class LogHelper` 定义（来自 BatchService.Framework.Utility 包），但 NLog 本身支持 `Warn`，常见 wrapper 应有。**必须确认并使用 `LogHelper.Warn(...)`**（如果没有这个方法，需要同时提一个补丁给 wrapper）

**问题 2 — "请检查上层为何未先查重"是给人看的，但没有触达机制**：
- 即使 level 改成 Warn，生产环境默认 `nlog.config` 的 target 是什么？从 `sbcLabSystem/bin/NLog.config` 和 `sbcLabSystem.Service/bin/Debug/NLog.config` 的存在可知有配置，但未检查具体 rule。若 target 只是写文件，仍然没有主动告警
- 本次 commit 作为"防重复"的缓解手段严重依赖这条日志被人看到，但实现上形同虚设

**修复建议**：
1. **必改**：`LogHelper.Info(...)` → `LogHelper.Warn(...)`（或等价方法）
2. **次要**：在 commit 或 CLAUDE.md 里补一句"生产 nlog.config 的 Warn 级 target 指向 xxxx，确保运维能收到告警"
3. **可选**：命中次数 + 时间戳汇总到一个独立表/文件，定期由运维脚本扫一次；这个比改 nlog.config 更容易控制

**评分影响**：-1.5 于"安全"维度（从 9.5 到 8.0）。

---

## 10. 必须修复的问题清单

按优先级排序。**只有 P0 全部修掉才能复评到 9.0+**。

### P0 — Critical，必须立即修复

1. **`QCDistributionRegisterInfoViewModel.ToEntityByLabCode` 的 ProjectId 过滤 (`cs:73`) 导致 `ConfigrmFee` / `ConfigrmEMS` / `switchNextOne` 全部回归**
   - **证据**：`sbcLabSystem/Views/Backstage/QCDistributionLabs.cshtml:91,108` 的对话框初始化字面量不含 ProjectId；`cshtml:385-415`（confirmFee 对话框）和 `cshtml:418-448`（confirmEMS 对话框）的 HTML 无 ProjectId 输入控件
   - **修复**：采纳 §6.2 方案 (C)
     ```csharp
     var query = _qcService.GetQcDistributionRegisters()
         .Where(p => p.QCDistributionId == regInfoivewModel.QCDistributionId
                  && p.LabId == labId);
     if (regInfoivewModel.ProjectId > 0)
     {
         query = query.Where(p => p.ProjectId == regInfoivewModel.ProjectId);
     }
     QCDistributionRegisterInfo entity = query.FirstOrDefault();
     ```
   - **手测剧本**（修复后必跑）：
     1. 打开 QCDistributionLabs → 点"收费确认"对话框 → 输入一个真实 LabCode → 点确认 → 检查 DB `IsCharged / LetterNo / ChargeTime` 三个字段是否真的更新
     2. 同上但点"标本寄出确认" → 检查 `EMSNo / IsSendEMS / SendEMSTime`
     3. 同上但先点"收费确认"确认后再点"下一个" → 检查对话框内容是否切到下一条

2. **`MergeRegisterInfo` 对 string 字段的"非 null 才覆盖"规则在真实调用链下失效，会把已有 `LetterNo`/`ChargeRemark`/`EMSNo` 等清空为 ""**
   - **证据**：`sbcLabSystem.Service/QC/QCService.cs:1062-1071`；触发链路见 §7.2 反例
   - **修复**：把所有 `if (source.XxxField != null)` 改为 `if (!string.IsNullOrEmpty(source.XxxField))`，并在 CLAUDE.md 契约章节"MergeRegisterInfo 的合并规则"里补一句"string 字段为 null 或空字符串都不覆盖"
   - **影响面**：需要让调用方在"主动想清空某字段"时走 Id>0 直改路径（已 CLAUDE.md 约定，但要补充 string 版本）

### P1 — Important，强烈建议修复

3. **`QCDistributionInfoViewModel.ToEntity` 第一道防线命中时只同步 2 个字段，与第二道防线的 MergeRegisterInfo 规则不一致**
   - **证据**：`sbcLabSystem/Models/Backstage/QCDistributionInfoViewModel.cs:152-156` vs `sbcLabSystem.Service/QC/QCService.cs:1040-1053`
   - **修复**：让第一道防线直接把 `registerInfo` 的所有可覆盖字段写到 `existing`（通过调 `MergeRegisterInfo`，为此需把 `MergeRegisterInfo` 从 `private static` 提成 `internal static` 或 public，或在 QCService 上暴露一个 `SaveOrMerge(...)` 方法）。这样双保险才是真正的"同一规则执行两次"

4. **`LogHelper.Info("[WARN] ...")` 改成真正的 Warn 级别**
   - **证据**：`QCService.cs:1043`
   - **修复**：改成 `LogHelper.Warn(...)`（需确认 wrapper 提供该方法；若无，补一个 wrapper 方法）

### P2 — Suggestion，可选改进

5. **`QCDistributionInfoViewModel.ToEntity` 的 per-loop DB 查重优化**
   - **证据**：`cs:148-151` 循环内 `FirstOrDefault`
   - **建议**：循环外一次性 `.Where(p => p.QCDistributionId == viewModel.Id).ToList()` 物化到 `Dictionary<(int lab, int proj), QCDistributionRegisterInfo>`，循环内查 dict

6. **`MergeRegisterInfo` 的字段列表脆弱**
   - **建议**：加一段 XML 注释说明维护约定；或用反射/表达式树遍历 `[MergeWhen*]` attribute 标注，避免后续加字段漏改

7. **`CLAUDE.md` 契约章节补一条关于 NLog 级别的操作约定**：
   - "本仓库 `LogHelper.Warn` 的 target 配置位于 xxx，生产环境需确认 Warn 级别会被监控系统识别"

---

## 11. 建议（非强制）

1. **长期方向**：仍建议和产品沟通"加非聚集索引 `(QCDistributionId, LabId, ProjectId)`"（**不是唯一索引**，用户硬约束只禁了唯一索引）——对查重性能和未来数据量增长都有好处
2. **手测剧本文档化**：L3 任务 commit 应附带手测 checklist，本次未见。建议以后在 `docs/impl/` 下补一份
3. **考虑把 `CurrentQCDistRegisterInfo` 在 `openFeeWindow` / `openEMSWindow` 初始化时直接从 `LabList.find(l => l.LabCode() === 输入的code)` 取整行**（LabCode 输入后 blur 触发），这样对话框里天然有 ProjectId 了，配合 §10.P0.1 方案 (C) 可以演进到 (A)

---

## 附：关键文件行号索引

- `sbcLabSystem.Service/QC/QCService.cs:1032-1085` — `SaveQcDistributionRegister` + `MergeRegisterInfo`
- `sbcLabSystem/Models/Backstage/QCDistributionInfoViewModel.cs:135-170` — `ToEntity` 第一道防线
- `sbcLabSystem/Models/Backstage/QCDistributionRegisterInfoViewModel.cs:59-84` — `ToEntityByLabCode` ProjectId 过滤（Critical 回归源头）
- `sbcLabSystem/Controllers/BackstageController.cs:215-230` — `ConfigrmFee`（受影响）
- `sbcLabSystem/Controllers/BackstageController.cs:239-256` — `ConfigrmEMS`（受影响）
- `sbcLabSystem/Controllers/BackstageController.cs:263-298` — `ImportLabs`（已修复）
- `sbcLabSystem/Controllers/BackstageController.cs:299-311` — `switchNextOne`（受影响）
- `sbcLabSystem/Controllers/BackstageController.cs:1225-1235` — `OutputUnderTakenLabs` 列清理
- `sbcLabSystem/Views/Backstage/QCDistributionLabs.cshtml:88-111` — `openFeeWindow` / `openEMSWindow` 字面量（Critical 回归的前端源头）
- `sbcLabSystem/Views/Backstage/QCDistributionLabs.cshtml:181-245` — `QCDistRegisterInfo` ko 构造器
- `sbcLabSystem/Views/Backstage/QCDistributionLabs.cshtml:383-450` — 收费/EMS 对话框 HTML
- `sbcLabSystem/Controllers/UserUIController.cs:259-274` — `saveAnswerInfo`（`MergeRegisterInfo` 失效路径）
- `CLAUDE.md:113-132` — `QCDistributionRegisterInfo 保存契约`
- `sbcLabSystem.Import/Form1.cs:28-65` — 一次性迁移工具（绕开路径，不影响线上）