branch-code-reviewer

分支代码审查

使用 dxs CLI 对 Datex Studio 分支进行 AI 辅助代码审查。读取分支的更改，追踪受影响配置的依赖项，读取统一差异，并生成带有明确严重性标签和审查结论的结构化摘要。

参考资料

• ../datex-studio-shared/branch-setup.md — 未提供分支 ID 时如何选择分支

依赖项

• impact-analysis 技能 — 在步骤 4（维度 F）中，只要有更改的配置被 delete，就会调用此技能。它会运行 dxs source explore reverse-trace 并对调用方进行分类，这是判断删除操作是否安全的权威方法。不要手动模拟此过程，也不要使用正向追踪——正向追踪无法回答“是否仍有内容引用已删除的名称？”（参见步骤 2 中的局限性说明）。

前提条件

• Datex Studio 功能分支的分支 ID。

工作流

[Step 1: Gather Context]
Parallel:
  dxs source changes --branch <ID>
  dxs source workitems --branch <ID> --description --comments
        |
[Step 2: Trace Dependencies of Changed Configs]
For each significantly-changed or deleted config:
  dxs source explore trace <config_name> --branch <ID>
        |
[Step 3: Review Diffs]
dxs source changes --branch <ID> --with-diffs
        |
[Step 4: Analyze Each Change]
For each changed config, evaluate against six dimensions:
  A. Bugs
  B. Code Quality
  C. Security
  D. Performance
  E. Simplification Opportunities
  F. Alignment & Risk
        |
[Step 5: Deeper Dive (as needed)]
If diffs are ambiguous:
  dxs source explore config <config_name> --branch <ID>
        |
[Step 6: Produce Review Summary]
Branch Overview → Changes Table → Detailed Findings (severity-tagged) →
Questions for Developer → Verdict

步骤详情

步骤 1：收集上下文

并行运行以下两条命令——它们互不依赖：

dxs source changes --branch <ID>
dxs source workitems --branch <ID> --description --comments

第一条命令快速列出更改的配置。第二条命令提供更改意图（缺陷／功能）、负责人、迭代上下文，以及（通过 --comments）可能影响审查者如何理解更改的讨论串内容。

步骤 2：追踪更改配置的依赖项

dxs source explore trace <config_name> --branch <ID>

追踪结果会显示配置的正向依赖项——该配置引用了什么（数据源、流程、网格、对话框、后端流程），以及每项内容来自哪个库。

何时进行追踪：

• 已删除的配置 — 追踪它们以查看其依赖项。然后与其他配置的追踪结果交叉核对，检查是否仍有内容通过名称引用已删除的配置。
• 有重大更改的配置 — 在阅读差异之前，先进行追踪以了解结构（此编辑器会调用哪些流程？此网格使用哪些数据源？）。了解依赖范围后再阅读差异，效率会高得多。
• 跨库引用 — 追踪结果会显示配置何时依赖共享平台流程（例如来自 Utilities 的 crud_create_flow）或其他模块中的对话框。这些交界处最容易发生问题。

追踪更改的配置时，留意是否引用了同一分支中已删除的配置。如果更改配置的追踪结果仍列出刚删除的数据源或流程，这说明清理不完整——应标记出来，尽管删除安全性本身会在步骤 4 中由 impact-analysis 处理。

局限性： 追踪只显示正向依赖项，不显示反向依赖项。要检查删除某个配置是否安全，请追踪可能引用它的配置，并在其依赖项列表中查找已删除的名称。

步骤 3：审查差异

dxs source changes --branch <ID> --with-diffs

所有更改配置的完整统一差异。这是步骤 4 的主要输入。

步骤 4：分析每项更改

针对每项更改的配置，按以下六个维度进行评估。并非每个维度都适用于所有配置——没有内容可说明时可跳过，但跳过前应逐一考虑。

A. 缺陷

• 逻辑错误、差一错误、条件反转
• 未处理的边界情况（空值实体、空集合）
• 错误处理缺失（缺少 catch 块、未检查流程结果）
• 新引入的死代码或不可达路径

B. 代码质量

• 阅读代码能否清楚理解更改意图？
• 是否符合周边代码使用的模式？
• 是否存在不必要的重复？
• 变量名称是否有意义？
• 是否正确使用平台 API（Datex Studio 辅助工具、$utils、$flows）？

C. 安全性

• 表达式或模板中未经清理的用户输入
• 动态筛选表达式中的注入风险
• 参数、日志或返回载荷中的敏感数据泄露
• 新公开操作缺少授权检查

D. 性能

• 不必要的 API 调用或重复获取数据
• 数据源中的 N+1 查询模式
• 本可以通过投影（$select/$expand 配合 $select）解决，却使用了过大的负载
• 低效循环或重复的 DOM 操作

E. 简化机会

• 过于复杂、可以写得更清晰的表达式
• 多余的 null 检查或可选链
• 配置之间重复的逻辑，可以共享
• 未使用的参数或无效的配置属性

F. 一致性与风险

• 工作项一致性——此更改是否确实解决了所述 bug/功能需求？

• 修复层级——这是根因修复，还是放错层级的变通方案？在 UI 层重新映射（例如，在绑定中重写错误的 TypeId）可能会掩盖数据层 bug，而该 bug 会在实体的其他读取场景中再次出现。如果无法判断原本的意图，请在“向开发者提出的问题”中提出，而不是猜测严重程度。

• 范围蔓延——是否捆绑了无关更改？

• 回归风险——这是否可能破坏现有行为？

• 删除配置的安全性——删除的配置在其他地方是否确实未被使用？对于变更清单中 modification 为 delete 的任何配置，请使用该引用名称和分支 ID 调用 impact-analysis 技能。它会运行反向追踪并返回仍在调用该配置的列表。将结果纳入审查：

  • 没有调用方 → [OK] 可以安全删除。
  • 存在调用方，且已在同一分支中更新 → [INFO] 列出已更新的调用方以供参考。
  • 存在调用方，且未在此分支中更新 → [ISSUE]——删除会导致这些调用方在运行时出错。

  不要尝试用正向追踪来模拟此操作。正向追踪无法回答“还有哪些地方引用这个已删除名称？”（参见步骤 2 中关于限制的说明）。

步骤 5：深入检查（按需进行）

评估 diff 是否提供了足够的上下文，让你能够作出明确判断。如果不够，请阅读受影响配置中更大范围的连续代码段，以了解周边逻辑：

dxs source explore config <config_name> --branch <ID>

当单行更改看起来像是修复，但可能会引入只有查看周边代码才能发现的副作用时，这一点尤其重要。缺少上下文的 diff 可能会掩盖“修复”破坏其他功能的事实。

步骤 6：生成审查摘要

使用下方“输出格式”中的确切结构。严重程度标签是审查者应采取行动的主要信号。

输出格式

# Review — Branch <ID>

## Branch Overview
- **Branch:** <ID> (<status>)
- **Work Item:** <type> <NNNNNN> — <title>
- **Assigned to:** <Author>
- **Sprint:** <sprint name>

## Changes Table

| Config | Type | Action | Summary |
|---|---|---|---|
| `<name>` | <type> | <add/update/delete> | <one-line summary> |
| …      | …    | …                 | …                     |

## Detailed Findings

### Bugs
**`[ISSUE]` <headline>** (`<config>`)
<explanation, with inline code where helpful>

**`[WARNING]` <headline>** (`<config>`)
<explanation>

### Code Quality
…

### Security Concerns
…

### Performance
…

### Simplification Opportunities
…

### Work Item Alignment
…

### Scope
…

## Questions for Developer
1. <question>
2. <question>

## Verdict
**<Approve | Request Changes | Needs Discussion>** — <one-line rationale>.

严重程度标签

标签	含义
[ISSUE]	合并前必须修复
[WARNING]	应当修复；存在潜在问题
[INFO]	仅供参考，无需采取行动
[OK]	已审查，没有问题

对于重要维度（如安全性、一致性），值得标注 [OK]，这样读者就知道审查者确实检查过——未提及的维度会被理解为“跳过”，这传达的是另一种意思。

示例（节选）

## Detailed Findings

### Bugs

**`[ISSUE]` Duplicate error check in save flow (`custom_field_editor`)**
The new create path checks `result.reason` twice:

    if (result.reason) { /* show error */ } else { /* save option */ }
    if (result.reason) { /* show error AGAIN */ } else { ... }

If `result.reason` is truthy, the error dialog shows twice. Looks like a
refactor artifact — the option-saving logic was inserted in the middle,
duplicating the error branch.

**`[WARNING]` `save_result` is unused (`custom_field_editor`)**
`const save_result = await $flows.Utilities.crud_create_flow({...});` — the
result is captured but never checked. If the create fails silently, the UDF
is created but its initial option value is lost with no error shown.

### Security Concerns
**`[OK]`** No new injection vectors. The datasource filter uses a template
literal with `$datasource.inParams.custom_field_id`, which is a typed number
param — low risk.

## Questions for Developer
1. The TypeId 5→1 remap is applied at the binding level — is this a display
   fix, or does the API return the wrong TypeId and the data layer needs the
   fix instead?
2. `save_result` from creating the initial option value is never checked —
   is silent failure acceptable here?

## Verdict
**Request Changes** — the duplicate error handling will show two dialogs on
failure. The unchecked `save_result` is a secondary concern worth addressing.

提示

1. 先追踪，再查看差异。 了解配置引用了什么，会改变你阅读其差异的方式。花 5 分钟使用 explore trace，可以省下 1 小时，避免脱离上下文误读差异。
2. 严守严重性标准。 [ISSUE] 表示“必须修复”——仅用于真正的阻塞问题。滥用此标记会让审查者学会忽略它。
3. 不要复述差异。 差异已经摆在那里。你的任务是指出问题（或没有问题的原因），而不是转述代码。
4. 对齐度是价值最高的维度。 “此项更改实现了工作项的要求”是最重要的问题；其他所有维度都通过了，仍可能交付错误的内容。
5. 删除配置后，务必全面检查追踪结果。 漏掉一个引用，就可能导致生产环境出现运行时错误。如有疑问，请追踪最可能引用已删除名称的前 N 个配置。
6. 只有空白字符不同的差异只是噪声。 将其标为 [INFO]；不要让它冲淡发现部分的重点。
7. 无法判断是有意还是无意时，就提问。 有些发现取决于无法从差异中看出的意图——未检查的流程结果是否可以接受？UI 层的重新映射是有意为之，还是权宜之计？这类问题应放在给开发者的问题中，而不是猜测严重性后列入发现。错误地标记 [ISSUE] 会损害可信度；提出精准的问题才能得到答案。

常见错误

错误	修正方法
未先追踪受影响的配置就查看差异	先执行步骤 2——先了解上下文，再查看内容
把所有问题都标为 [ISSUE]	仅将 [ISSUE] 用于真正的阻塞问题；其他情况使用 [WARNING] / [INFO]
跳过工作项对齐检查	对齐度是最重要的维度——不要跳过
删除配置时不检查反向引用	对已删除的引用名称调用 impact-analysis；不要用正向追踪来模拟
更改较为隐晦时仅依赖差异	步骤 5：使用 dxs source explore config 获取完整配置，以查看周边逻辑
将空白字符或格式噪声逐项列为发现	只标记一次 [INFO]（或省略）——不要凑报告篇幅
对取决于未知意图的问题指定严重性	将其移至给开发者的问题中，而不是猜测