PR 审查验证
审查一个 PR 时,按 L0→L4 逐级验证,每级有则记无则跳,最终给"可合并 / 可合并但有建议 / 需返工 / 无法验证(写明缺什么)"结论。
先 L0 分诊(判是否适用)→ 适用 PR 做 L1/L2(总做、不依赖环境)→ 再按 PR 类型决定 L3/L4。 L0 判定 skip 的(前端/CI/纯 git 启动逻辑)不做 L1/L2。游戏流程类 PR,L4 live 是硬性要求(不是可选项)。
0. 准备:分支与基线(每个 PR 必做)
原则:在"PR + 当前 main"的集成结果上审查,不在旧 base 上审。 PR 基于旧 main,旧 base 上审会漏集成问题(API 不兼容、被 main 改过的同文件、依赖 main 新加的文件等)。
- 用提交者分支(
gh pr checkout <n>,拉的是 PR 当前 HEAD;不要用可能被本地 merge 污染的旧本地分支)。
- 先
git fetch origin main 再 git merge origin/main:fetch 确保 remote 引用最新(否则 merge 的是旧 origin/main,审查基线仍落后);merge 后确保改动在当前代码上成立——老 PR 不 merge 可能缺文件 / 跑崩,merge 后才有"这个 PR 真能合、合了不崩"的判断基线。
- merge 冲突 → 见 §6(先解冲突再审,解的过程本身也暴露集成影响)。
- 审完再决定改不改:看到 review comment(CodeRabbit / 人)不要先改代码——先在 merged 代码上审(理解改动 + 框架语义),再决定 comment 采纳(改)还是驳回(说明理由)。顺序:merge → 审 → 评 comment → 改。
- 每个 PR 开一个 notes,记:背景核实 / 改动合理性 / 每级验证结果 / 结论 / 给 reviewer 的要点。
- 测试仓也必须切到 PR 同名分支(和主仓
gh pr checkout 对应):处理每个 PR 前,无论该 PR 有无配套测试仓 PR,都先在测试仓确保有同名分支——git -C zzz-od-test checkout <PR 同名分支>(无则 checkout -b 本地新建,不必等测试 PR),再 git -C zzz-od-test fetch origin && git merge origin/main(和主仓一样,确保测试改动在最新测试仓 main 上成立)。PR 的测试改动(新测试 / 截图 fixture)只进该分支,绝不直接 commit/push 测试仓 main。测试改动走 git -C zzz-od-test(主仓 gitignore 会静默跳过)。
- 先建分支的目的:人一上来就在正确分支,后续补测试时不会忘记切分支 → 误 commit 测试仓
main(就是 #2348 的 4ca301d 教训:测试直接进 main → 所有 PR test-check 红)。哪怕该 PR 暂时没测试,也先把分支建好占位。
- 为什么:测试仓
main 是 test-check 的基准,必须与主仓 main 同步。若把未合 PR 的测试直接合到测试仓 main → 测试仓 main 领先主仓(测了还不存在的代码)→ 所有 PR 的 test-check 全红(实测:#2348 的测试 4ca301d 误合测试仓 main,致 main 自己 + 所有后续 PR 的 test-check fail)。正确时序:该 PR 合进主仓 main 后,再把它的测试分支合进测试仓 main。
- 截图 fixture 路径契约:与
zzz-od-dev-screen-onboarding 一致,screens/<screen_name>/<state>.webp(screen_name 目录 + 可读状态名 + .webp,如 战斗画面/默认.webp、战斗画面/精英.webp);勿用 .png 或时间戳文件名。
1. L0 分诊
前端 / CI / 纯 git 启动逻辑 → 一般 skip(记原因)。业务逻辑 / 画面识别 / 操作流程 → 继续。
游戏流程类 PR(上句判"继续"的),L1 静态审查前先建玩法理解:先查 docs/game/gameplay/(玩法机制)+ docs/game/screens/(画面建档)有无相关玩法 / 画面建档 —— 有则先读,建立「这玩法是什么、目标 / 资源 / 循环、走哪些画面」的初步理解;无则先查玩法(网络攻略 / wiki / 实玩)。带着玩法理解再看 PR 代码,能更快判断改动合理性、识别流程缺失 / 画面未建模(比直接读 diff 高效,避免审到一半才补查)。非游戏流程 PR 跳过本步。
2. L1 静态审查(总是做)
读 diff + 相关代码。查:逻辑错误、回归、死循环、资源泄漏、是否复用现有模式(Application / Operation / config / setting card)、类型注解、1080p 硬编码合理性。
框架语义必查项(任一错就运行时崩,逐条对照源码确认):
- 生命周期钩子
after_operation_done / op_callback:在 success 和 fail 都会触发(在结果判断之外)。必查:若 PR 在此做了自定义清理(如 stop_xxx),该清理有几个子步骤?任一步骤抛异常的概率?若有异常风险 → 基类调用必须在 finally 里(try 包自定义清理,finally 包基类清理),否则 run_record/notify/APPLICATION_STOP 等基类清理被跳过。这是"代码看着对但异常路径漏"的高发点,不要只看 happy path。
- 节点重试预算:
round_wait 重置 node_retry_times(无界),只有 round_retry 消耗、超 node_max_retry_times(默认 3)才 FAIL。审"删除大 node_max_retry_times"的重构时,确认主路径走 round_wait(否则默认 3 不够)。
@operation_node 装饰器:只挂元数据、原样返回 func → 可直接调用被装饰方法;节点调度读元数据。删/改节点不影响直接调用。execute() 重复调用安全:每次开头全重置(节点图 + 重试计数 + handle_init)。retry 包 execute 的模式安全。- 节点路由
node_from(status=...):契约从"抛异常"改"返回 None/默认值"时,逐个消费方确认 None-safe(否则只是把崩溃换地方)。ignore_status + status 组合决定路由,有疑义查匹配源码。
3. L2 背景核实
PR 描述 / 关联 issue 提到的游戏行为 / bug 是否属实?对 assets/game_data/screen_info/(画面模型)、docs/game/(玩法)、相关代码路径交叉验证。issue 标题/日志往往透露真实复现路径(如"某 app 复用同一 operation"会扩大影响范围)。
4. L3 离线运行(不改游戏状态)
- 用测试仓留档截图喂
analyze_screen,验画面识别 / OCR 是否如改动静称。
- 自定义 OCR / 识别管线(颜色 mask + ROI 切分等):写脚本用项目 ctx 在截图上复刻该管线再断言(比整图默认 OCR 强,能抓串扰)。
- PR 自带测试:直接
pytest 跑。
5. L4 live 运行(游戏流程类 PR 硬性要求)
想办法在游戏里验证,优先可逆、低消耗路径:
- 导航到目标画面(
click_game / 通用 op 如 GotoMenu)→ capture_game_screen / analyze_screen 在实时截图上验识别/行为。
- 避开消耗周限/体力/日限的动作与"停止托管"等不可逆按钮;导航类(菜单/仓库/快捷手册)对后台托管无害。
- server 跑当前检出分支的代码:要验某 PR 流程 → checkout(先 merge main)→ 经 daemon 重启 server → server 即该分支逻辑 →
run_operation / run_standalone_app 驱动 + get_run_status 观察。
- 大世界等需解锁光标的画面,
click_game 传 pc_alt=true;子画面/菜单 false。
- 沿途画面若 screen_info 未建模 → 顺便建档(补 area/screen_info;走
zzz-od-dev-screen-onboarding)。
- 真无法 live(纯消耗且无低消耗切入 / 需特定账号或地区 / 需外部凭据)→ 明确标"无法 live 验证"并写明缺什么,不要假装验过。
6. 冲突解决(merge origin/main 时)
- 看实际冲突再判断,别预设复杂:很多"看似游戏流程"的冲突其实机械可解(注释 / import / 不同区域 additive)。
- 逻辑纠缠(同文件两侧都改核心)→ 先 live 建档该玩法(搞清当前真实结构),再据此融合,不盲合。
- 解完必验证三连:
py_compile + ruff + import 冒烟(import 比 compile 强,抓运行时导入/符号缺失)。
7. 结论与产出
每 PR 给明确结论 + reviewer 要点。性能 / 过程建议即便非阻塞也记(给 maintainer 决策)。
PR 收尾(处理 review comment、推到可合并)走 zzz-od-dev-pr-finishing;单条 review comment 处理走 superpowers:receiving-code-review。
通用原则
- 追源码确认,别照 description 行事:框架语义(op_callback 触发时机、round_wait/retry 预算、路由匹配)以源码为准,PR description 可能省略。
- 删配置项/重构类:grep 全量残留引用 + 确认迁移完备 + 测试仓同步检查。
- 巨型 PR:抽查高风险模块(有测试的优先 L3 实跑),诚实标注未深审范围,建议拆分而非整体背书。
- 性能:涉及热路径(战斗循环/OCR 推理)的改动,看锁粒度与阻塞——
Future.result() 同步等、推理期持锁串行化,默认配置下影响所有用户时尤其要标。
- PR 合并后:可提示用户删该 PR 的本地 + remote 分支(
git branch -d <branch> + git push origin --delete <branch>)——只提示,不主动做(分支可能还在用 / 由用户决定时机)。详见 zzz-od-dev-pr-finishing §5。
1---2name: zzz-od-dev-pr-review3description: 当要审查/验证一个 open PR 是否可合并时用。英文 review/verify a PR、check if mergeable、validate PR functionality、PR code review。4---56# PR 审查验证78审查一个 PR 时,按 L0→L4 逐级验证,每级有则记无则跳,最终给"可合并 / 可合并但有建议 / 需返工 / 无法验证(写明缺什么)"结论。9**先 L0 分诊(判是否适用)→ 适用 PR 做 L1/L2(总做、不依赖环境)→ 再按 PR 类型决定 L3/L4。** L0 判定 skip 的(前端/CI/纯 git 启动逻辑)不做 L1/L2。游戏流程类 PR,L4 live 是硬性要求(不是可选项)。1011## 0. 准备:分支与基线(每个 PR 必做)1213**原则:在"PR + 当前 main"的集成结果上审查,不在旧 base 上审。** PR 基于旧 main,旧 base 上审会漏集成问题(API 不兼容、被 main 改过的同文件、依赖 main 新加的文件等)。1415- 用提交者分支(`gh pr checkout <n>`,拉的是 PR 当前 HEAD;不要用可能被本地 merge 污染的旧本地分支)。16- **先 `git fetch origin main` 再 `git merge origin/main`**:fetch 确保 remote 引用最新(否则 merge 的是旧 `origin/main`,审查基线仍落后);merge 后确保改动在当前代码上成立——老 PR 不 merge 可能缺文件 / 跑崩,merge 后才有"这个 PR 真能合、合了不崩"的判断基线。17- merge 冲突 → 见 §6(先解冲突再审,解的过程本身也暴露集成影响)。18- **审完再决定改不改**:看到 review comment(CodeRabbit / 人)不要先改代码——先在 merged 代码上审(理解改动 + 框架语义),再决定 comment 采纳(改)还是驳回(说明理由)。顺序:merge → 审 → 评 comment → 改。19- 每个 PR 开一个 notes,记:背景核实 / 改动合理性 / 每级验证结果 / 结论 / 给 reviewer 的要点。20- **测试仓也必须切到 PR 同名分支**(和主仓 `gh pr checkout` 对应):处理每个 PR 前,**无论该 PR 有无配套测试仓 PR,都先在测试仓确保有同名分支**——`git -C zzz-od-test checkout <PR 同名分支>`(无则 `checkout -b` 本地新建,不必等测试 PR),再 `git -C zzz-od-test fetch origin && git merge origin/main`(和主仓一样,确保测试改动在最新测试仓 main 上成立)。PR 的测试改动(新测试 / 截图 fixture)**只进该分支,绝不直接 commit/push 测试仓 `main`**。测试改动走 `git -C zzz-od-test`(主仓 gitignore 会静默跳过)。21 - **先建分支的目的**:人一上来就在正确分支,后续补测试时不会忘记切分支 → 误 commit 测试仓 `main`(就是 #2348 的 `4ca301d` 教训:测试直接进 main → 所有 PR test-check 红)。哪怕该 PR 暂时没测试,也先把分支建好占位。22 - **为什么**:测试仓 `main` 是 test-check 的基准,必须与主仓 `main` 同步。若把**未合 PR** 的测试直接合到测试仓 `main` → 测试仓 `main` 领先主仓(测了还不存在的代码)→ **所有 PR 的 test-check 全红**(实测:#2348 的测试 `4ca301d` 误合测试仓 `main`,致 `main` 自己 + 所有后续 PR 的 test-check fail)。正确时序:该 PR 合进主仓 `main` 后,再把它的测试分支合进测试仓 `main`。23 - **截图 fixture 路径契约**:与 `zzz-od-dev-screen-onboarding` 一致,`screens/<screen_name>/<state>.webp`(screen_name 目录 + 可读状态名 + `.webp`,如 `战斗画面/默认.webp`、`战斗画面/精英.webp`);勿用 `.png` 或时间戳文件名。2425## 1. L0 分诊2627前端 / CI / 纯 git 启动逻辑 → 一般 skip(记原因)。业务逻辑 / 画面识别 / 操作流程 → 继续。2829**游戏流程类 PR(上句判"继续"的),L1 静态审查前先建玩法理解**:先查 `docs/game/gameplay/`(玩法机制)+ `docs/game/screens/`(画面建档)有无相关玩法 / 画面建档 —— 有则先读,建立「这玩法是什么、目标 / 资源 / 循环、走哪些画面」的初步理解;无则先查玩法(网络攻略 / wiki / 实玩)。带着玩法理解再看 PR 代码,能更快判断改动合理性、识别流程缺失 / 画面未建模(比直接读 diff 高效,避免审到一半才补查)。非游戏流程 PR 跳过本步。3031## 2. L1 静态审查(总是做)3233读 diff + 相关代码。查:逻辑错误、回归、死循环、资源泄漏、是否复用现有模式(Application / Operation / config / setting card)、类型注解、1080p 硬编码合理性。3435**框架语义必查项**(任一错就运行时崩,逐条对照源码确认):36- **生命周期钩子 `after_operation_done` / `op_callback`**:在 success 和 fail 都会触发(在结果判断之外)。**必查**:若 PR 在此做了自定义清理(如 stop_xxx),该清理有**几个子步骤**?任一步骤抛异常的概率?若有异常风险 → **基类调用必须在 `finally` 里**(try 包自定义清理,finally 包基类清理),否则 run_record/notify/`APPLICATION_STOP` 等基类清理被跳过。这是"代码看着对但异常路径漏"的高发点,不要只看 happy path。37- **节点重试预算**:`round_wait` 重置 `node_retry_times`(无界),只有 `round_retry` 消耗、超 `node_max_retry_times`(默认 3)才 FAIL。审"删除大 `node_max_retry_times`"的重构时,确认主路径走 `round_wait`(否则默认 3 不够)。38- **`@operation_node` 装饰器**:只挂元数据、原样返回 func → 可直接调用被装饰方法;节点调度读元数据。删/改节点不影响直接调用。39- **`execute()` 重复调用安全**:每次开头全重置(节点图 + 重试计数 + `handle_init`)。retry 包 `execute` 的模式安全。40- **节点路由 `node_from(status=...)`**:契约从"抛异常"改"返回 None/默认值"时,**逐个消费方确认 None-safe**(否则只是把崩溃换地方)。`ignore_status` + `status` 组合决定路由,有疑义查匹配源码。4142## 3. L2 背景核实4344PR 描述 / 关联 issue 提到的游戏行为 / bug 是否属实?对 `assets/game_data/screen_info/`(画面模型)、`docs/game/`(玩法)、相关代码路径交叉验证。issue 标题/日志往往透露真实复现路径(如"某 app 复用同一 operation"会扩大影响范围)。4546## 4. L3 离线运行(不改游戏状态)4748- 用测试仓留档截图喂 `analyze_screen`,验画面识别 / OCR 是否如改动静称。49- **自定义 OCR / 识别管线**(颜色 mask + ROI 切分等):写脚本用项目 ctx 在截图上复刻该管线再断言(比整图默认 OCR 强,能抓串扰)。50- PR 自带测试:直接 `pytest` 跑。5152## 5. L4 live 运行(游戏流程类 PR 硬性要求)5354**想办法在游戏里验证**,优先可逆、低消耗路径:55- 导航到目标画面(`click_game` / 通用 op 如 GotoMenu)→ `capture_game_screen` / `analyze_screen` 在**实时截图**上验识别/行为。56- 避开消耗周限/体力/日限的动作与"停止托管"等不可逆按钮;导航类(菜单/仓库/快捷手册)对后台托管无害。57- **server 跑当前检出分支的代码**:要验某 PR 流程 → checkout(先 merge main)→ 经 daemon 重启 server → server 即该分支逻辑 → `run_operation` / `run_standalone_app` 驱动 + `get_run_status` 观察。58- 大世界等需解锁光标的画面,`click_game` 传 `pc_alt=true`;子画面/菜单 `false`。59- **沿途画面若 screen_info 未建模 → 顺便建档**(补 area/screen_info;走 `zzz-od-dev-screen-onboarding`)。60- 真无法 live(纯消耗且无低消耗切入 / 需特定账号或地区 / 需外部凭据)→ 明确标"无法 live 验证"并写明缺什么,**不要假装验过**。6162## 6. 冲突解决(merge origin/main 时)6364- **看实际冲突再判断,别预设复杂**:很多"看似游戏流程"的冲突其实机械可解(注释 / import / 不同区域 additive)。65- 逻辑纠缠(同文件两侧都改核心)→ **先 live 建档该玩法**(搞清当前真实结构),再据此融合,不盲合。66- 解完必验证三连:`py_compile` + `ruff` + **`import` 冒烟**(import 比 compile 强,抓运行时导入/符号缺失)。6768## 7. 结论与产出6970每 PR 给明确结论 + reviewer 要点。性能 / 过程建议即便非阻塞也记(给 maintainer 决策)。71PR 收尾(处理 review comment、推到可合并)走 `zzz-od-dev-pr-finishing`;单条 review comment 处理走 `superpowers:receiving-code-review`。7273## 通用原则7475- **追源码确认,别照 description 行事**:框架语义(op_callback 触发时机、round_wait/retry 预算、路由匹配)以源码为准,PR description 可能省略。76- **删配置项/重构类**:grep 全量残留引用 + 确认迁移完备 + 测试仓同步检查。77- **巨型 PR**:抽查高风险模块(有测试的优先 L3 实跑),诚实标注未深审范围,建议拆分而非整体背书。78- **性能**:涉及热路径(战斗循环/OCR 推理)的改动,看锁粒度与阻塞——`Future.result()` 同步等、推理期持锁串行化,默认配置下影响所有用户时尤其要标。79- **PR 合并后**:可**提示**用户删该 PR 的本地 + remote 分支(`git branch -d <branch>` + `git push origin --delete <branch>`)——只提示,**不主动做**(分支可能还在用 / 由用户决定时机)。详见 `zzz-od-dev-pr-finishing` §5。
onedragon-anything