zzz-od-dev-pr-review

name	zzz-od-dev-pr-review
description	当要审查/验证一个 open PR 是否可合并时用。英文 review/verify a PR、check if mergeable、validate PR functionality、PR code review。

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。