| name | spec-branch-review |
| description | 审查一个功能分支的实现质量:先建立判据(有需求文档则读原文,没有则从 spec/commit/PR/注释/测试/调用方/仓库惯例逐层重建),再实测缺陷而非静态猜测,最后按可用判据出维度结论。当用户要求「审查这个分支」「review 这次实现」「看看代码实现情况」时使用,无论有无需求文档。只做审查与定级,不做修复。 |
定位
审查已完成的功能分支:它是否真的做到了该做的事,代码本身站不站得住,AI 和流程用得对不对。
产出是判断,不是修复。除非用户明确要求,不改一行业务代码。
不适用:单文件 diff 速览、PR 评论逐行标注、纯安全审计(走 security-reviewer)。
铁律
- 先建立判据,再看代码。 没有判据就没有「符合/不符合」,只有「我觉得」。判据首选需求原文(给了链接就读完,含被截断的表格和附件);没有需求文档时不要跳过这一步,改用「判据重建」自己造一份,见 §1b。
- 每个缺陷必须实测复现。 静态读代码只能产出「可能有问题」。写一个探针脚本跑出来,把输出贴进报告。做不到实测的标
[INFERENCE]。
- 只读,不写。 用
git archive 导出到临时目录做验证,不用 git worktree、不 checkout、不改工作区。项目若有 git 红线规则(多数 spec 驱动项目有),任何写类 git 操作都要先经用户批准。
- 用它自己的尺子量它。 分支内 spec 的「验收标准」章节是最有力的判据。自己定的标准没达成,比外部标准没达成更硬。
- 区分存量与本次引入。 每个缺陷都要
git show master:<file> 对一次。存量问题不该算在这次交付头上,本次引入的回退要重点标。
流程
1. 定位分支
分支常常不在本地:
git ls-remote --heads origin
git fetch origin <branch>
git log --oneline master..origin/<branch>
git diff --stat master...origin/<branch>
飞书需求文档(先读 skill://lark-doc):
lark-cli docs +fetch --doc "<url>" --doc-format markdown --as user --format json
长文档会被截断。返回里出现 [Some lines truncated] 或表格只剩表头时,用 block 定位补读:
lark-cli docs +fetch --doc "<url>" --scope keyword --keyword "关键词" --format json
lark-cli docs +fetch --doc "<url>" --scope range \
--start-block-id <id> --end-block-id <id> --format json
需求正文里的 <cite> 标签指向附件规范(量规、指标定义),是验收口径的一部分,要跟着读。
1b. 判据重建(无需求文档时必做)
没有需求文档是常态,不是例外。此时判据由审查人自己重建,按可信度排序取用 —— 越靠前越接近「作者当时的承诺」,越靠后越接近「我的偏好」。绝不能因为缺需求就直接跳到读代码风格。
| 序 | 判据来源 | 怎么取 | 可信度 |
|---|
| 1 | 分支内 spec / plan / 设计文档 | git diff --name-only master...<br> 里的 .md | 最高:作者自己写的承诺 |
| 2 | commit message 的 body | git log --format="%h %s%n%b" master..<br> | 高:声明了意图与范围 |
| 3 | PR / issue 描述 | read pr://<N>、read issue://<N> | 高 |
| 4 | 代码注释与 docstring | 改动处的注释说了函数该做什么 | 中:常与实现脱节,脱节本身就是发现 |
| 5 | 新增/修改的测试 | 测试名与断言反映作者认为的正确行为 | 中 |
| 6 | 被修改函数的既有调用方 | lsp references | 中:调用方的假设就是隐含契约 |
| 7 | 同仓库同类模块的既有惯例 | 邻近模块怎么做的 | 中低:一致性判据 |
| 8 | 语言/框架通用正确性 | 见下方清单 | 兜底:与项目无关,永远成立 |
关键动作:把重建出的判据写进报告开头,并声明它是重建的。
本次审查无需求文档。判据重建自:commit body 声明的「修复 X」+ analyzer.py 改动处 docstring + 3 条新增测试的断言。以下「符合/不符合」均以此为准,非业务需求验收。
这一句让用户能立刻纠正你 —— 判据错了,结论再漂亮也没用。
判据仍不足时:改问「这次改动自身是否自洽」
连 commit body 都是空的,就放弃「是否满足需求」这一问,换成三个不依赖外部判据、且永远可查的问题:
- 改动是否自洽 —— 动了 A 却没动与 A 耦合的 B?(取值域、映射表、调用方、序列化两侧、文档与实现)
- 改动是否有回退 —— 删掉的旧代码防的是什么?
git show master:<file> 读原注释与原实现,被删的防御往往有历史原因
- 改动是否可验证 —— 新行为有没有测试或可复现的验收记录?没有就是「未验证交付」,本身即缺陷
这三问全部只靠 diff + master 对比就能回答,不需要任何需求文档。实践中最高产的两个 P0 级发现都来自第 1 问。
无判据时也永远成立的检查清单
这些与业务需求无关,可以直接当判据用:
- 资源泄漏 —— 打开的文件/连接/客户端有没有
finally 或 context manager
- 异常吞没 ——
except: pass、except Exception: break 是否吃掉了本该冒泡或重试的错误
- 错误信息与实际原因不一致 —— 内层
raise 被同层 except 覆盖
- 边界值 —— 空集合、单元素、None、0、负数、超长输入
- 并发/重入 —— 模块级可变状态、跨请求共享实例
- 静默降级 —— 失败后写入默认值继续跑且不告警(比抛异常更难排查)
- 硬编码与魔数 —— 尤其是两处数值互相耦合却无常量(如
450 对 [:500])
- docstring 与实现不符 —— 声称「一级标题」而正则是
#{1,6}
- 安全 —— 拼接 SQL/命令、路径穿越、日志打印密钥
需要用户拍板时才问
重建判据时若遇到两种解读都合理、且结论相反的情况,不要自己选一个然后当成事实。列出两种解读、各自的结论差异,问用户。例如「level=0 是有意的哨兵值还是漏改」—— 前者不是缺陷,后者是 P0。
这跟「缺需求就问用户要需求」不同:前者是穷尽了 8 层判据后剩下的真歧义,后者是偷懒。
2. 建立只读验证环境
git archive --format=zip -o /tmp/br.zip origin/<branch>
解压时跳过大二进制(课标 PDF、音视频、样本 docx 能有几百 MB,会让解压超时):
skip = ('.pdf', '.mp4', '.wav', '.docx')
for info in zf.infolist():
if info.filename.lower().endswith(skip): continue
zf.extract(info, dst)
跑一次分支自带的测试,核对 spec 里声称的数字:
cd <tmpdir> && python -m pytest <该分支新增的测试文件> -q
数字不符就是第一个红旗。
3. 读 spec 文档,提取判据(有则必读)
git diff --name-only -z master...origin/<branch> | tr '\0' '\n' | grep '^spec/'
git show "origin/<branch>:<spec路径>" > /tmp/s1.md
(git diff --name-only 对非 ASCII 路径会输出八进制转义,用 -z + tr 拿原始路径。)
每份 spec 抓四样东西:
- 非目标 / 不做清单 — 划定「不该批评什么」的边界
- 验收标准 — 逐条核对的判据,最硬的尺子
- 决策记录 — 范围裁剪有没有留痕、有没有跟业务确认过
- 验证记录 — 声称的测试数、真实样本、验收产物路径
分支里没有 spec 文档,就退回 §1b 的判据阶梯 —— 此时「Spec 工作流」维度不打分,改为记一句「本次无 spec 文档,该维度不适用」,不要因为缺 spec 就扣分(很多项目本就不用 spec 驱动)。
4. 逐项核对判据
做一张三列表:条目 / 状态 / 核查依据。「条目」来自需求文档,或来自 §1b 重建的判据。状态只用三种:
| 状态 | 含义 |
|---|
| 已实现 | 找到了代码依据,且实测有效 |
| 明确不做 | spec 决策记录或 commit body 里写了,附出处 |
| 未覆盖 | 判据要求但既没实现也没声明不做 —— 这是真问题 |
「明确不做」不算缺陷,但要在结论里提醒:裁剪后无法满足全部验收口径,建议在 PR 里列未覆盖清单。
判据是重建的,表格标题要写明「判据(重建)逐项核对」,不要伪装成需求验收。
5. 实测缺陷
D:\tmp\probe_*.py 这类独立探针脚本,一个问题一个:
import sys, os
sys.path.insert(0, r"<tmpdir>")
os.chdir(r"<tmpdir>")
print("KEY", value)
跑法(避免 kernel 卡死,用 bash 跑脚本文件而不是塞进 eval 的长 code):
cd <tmpdir> && python /tmp/probe_x.py 2>&1 | grep -E "^(KEY|OTHER)"
高产的探测方向,按命中率排序:
- 取值域收窄 — 改了
1-5 → 2-5,那么所有与这个域耦合的东西都要查:索引表 [...][level-1]、if x in ["1","2"] 判断、映射字典、兜底默认值。这类改动极少只改一处就完整。
- 删除防御代码 — 被删的兜底/降级分支,注释里往往写着它防的是什么。
git show master:<file> 读原注释:写着「避免 X」的,删掉后 X 就回来了。
- 异常与重试的接缝 — 新抛的异常有没有被外层
except ... : break 吞掉?把重试循环连同 fake LLM 一起复刻,数实际调用次数。
- 预算/截断链 — 多处
[:N] 串联时,前一段吃满预算会让后面的内容全丢。构造超长输入实测每一类材料还在不在。
- 正则的作用范围 —
^#{1,6} 这类量词是否比 docstring 声称的范围更宽?喂一个嵌套结构看会不会误伤。
- 报错信息与实际原因是否一致 — 内层
raise ValueError 被同层 except ValueError 覆盖,是最常见的排障误导。
6. 维度定级
维度按可用判据裁剪,不硬凑四项。 没有需求文档时「需求实现情况」改叫判据符合度(标注判据来源);没有 spec 文档时「Spec 工作流」整项写「不适用」。宁可只给两个维度加一句说明,也不要为凑格式编出无依据的评分。
刻度定义(给分必须落在这张表上)
满分 A,最低 D。四档,不设 A+ / F;± 只表示「该档内偏上/偏下」,不是独立档位。
每个字母必须配一句可核查的事实依据(缺陷数、变异捕获率、实测命中项)。给不出事实就降一档 —— 说不清为什么是 A,它就不是 A。
| 档 | 通用含义 | 合并含义 |
|---|
| A | 该维度无缺陷,且有超出基本要求的主动作为 | 直接可合 |
| B | 方向正确、主体达成,存在不阻塞的疏漏 | 修完必修项可合 |
| C | 达成一半,或有 1 个以上阻塞缺陷 | 打回补做 |
| D | 未达成,或引入比修复前更差的行为 | 拒收 |
各维度的降档触发条件(命中一条即从 A 落到对应档,取最低者):
| 维度 | A | B | C | D |
|---|
| 需求实现 / 判据符合度 | 判据逐条已实现,无「未覆盖」 | 有「明确不做」且留痕 | 有「未覆盖」条目,或裁剪未留痕 | 声称完成但核心条目未实现 |
| 代码编写质量 | 零本次引入缺陷;变异测试全捕获 | 仅 P1/卫生问题 | 有 1 个以上 P0 | 引入功能回退或出参错误 |
| AI 使用情况 | 真模型真样本,且从真实运行挖出假设外缺陷 | 真样本验收但只覆盖 happy path | 仅 mock 验收 / 主要断言 prompt 字符串 | 无任何验收记录 |
| Spec 工作流 / 交付完备性 | 角色文档齐备、决策留痕、经验已沉淀 | 缺 summary 或经验未沉淀 | status 倒挂、计划与实际无对账 | 无 spec 且无 commit 说明 |
「没做错」只到 B 档上限。 A 要求主动作为的证据,例如:反转了守旧的假绿用例、修了测试自身的盲区、变异测试双向验证(既测「宽了能过」也测「别宽过头」)、拒绝某个改动并写下理由。
「代码编写质量」不因存量问题降档 —— 存量单列,只在报告里提一句。
各维度看什么
需求实现情况 / 判据符合度 — 逐条核对结果。看「未覆盖」有几条,「明确不做」是否留痕。判据为重建时,明确写出重建来源与它的可信度层级。
代码编写质量 — 缺陷数与严重度;影响面是否收得住(契约有没有乱改);是否守住了跨模块边界;代码卫生(区分存量/本次)。
AI 使用情况 — 判断标准是验收是否跑了真东西:
- 真模型 + 真样本 → 高分;只断言 mock 被调用 → 低分
- 有没有从真实运行里挖出假设外的缺陷(这是最强信号)
- 缺陷修在根因层,还是继续调 prompt 求模型别犯错(前者对)
- 测试是否在断言 prompt 字符串 ——
assert "某文案" in PROMPT 只证明文案改了,不证明行为对了;这类断言占比过高说明测试没测行为
- 编排层(重试/降级/落盘)有没有覆盖 —— 解析函数测得再全,编排层缺陷照样漏
Spec 工作流(仓库使用 spec 驱动时) — 跟仓库既有 spec 对比,不要凭空立标准:
git ls-tree -r --name-only -z master "spec/03-功能实现/<某个已完成的spec>/" | tr '\0' '\n'
看差什么:角色子目录(explorer/ writer/ tester/ executor/)、summary.md、test-report.md、frontmatter 完整度、wikilink 关联、spec/context/experience/ 有没有沉淀。
另查两处容易漏的:
- status 倒挂 — 上游设计标「待确认」而下游实现标「已验收」
- 经验沉淀 — 本次踩的坑是否与
.agents/rules/ 或已有 EXP 同源却没触发那条检查
仓库根本不用 spec 驱动 → 这一维度换成交付完备性:改动有没有配套测试、有没有可复现的验证记录、commit message 说不说得清意图与范围。
7. 出结论
默认交付形态是对话内结论,不产出文件。 审查结论常与用户预期相反(「你以为能合,实测不能」),
先在对话里给出,用户才能立刻纠正判据——判据错了,再漂亮的报告都得重做。
先给合并判断,再给理由。阻塞项只放两类:
- 没通过它自己写的验收标准(或重建判据里可信度最高那层的承诺)
- 本次引入的功能回退或出参错误
不放进阻塞项:存量问题、留痕过的范围裁剪、代码卫生、spec 收尾。这些进「合并前顺手清」或「下个 spec」。
修复优先级表用四档:合并前必修 / 合并前建议 / 收尾 / 对齐业务。
用户只做审查时,不要给修复代码。 给出根因、位置、修法一句话,到此为止。
8. 问是否要 HTML 报告
对话结论给完后,主动问一句是否需要 HTML 报告,不要默认生成、也不要默认不提。
首轮:「需要我把这份审查整理成 HTML 报告吗?」
复审:先 read docs/reports/ 看该分支是否已有报告 —— 有就问「需要我把这轮复审追加到既有报告里吗?」
(追加 section,不是新建文件)。
值得提的场景:结论要转给他人、缺陷条目多需要分档呈现、要留档对比后续复查。
用户没要就到此结束——审查报告是一次性产出,不是仓库资产,默认写文件只会堆无人维护的文件。
HTML 报告(仅在用户同意后执行)
本节是 §8 得到肯定答复后才走的分支。
一个分支一个文件,每轮追加 section
同一分支的复审绝不新建文件。 每轮审查往既有报告里追加一个
<section class="round">,用顶部标签页切换。理由:分支是同一个,读者要的是「这轮比上轮好在哪」,
拆成多文件就得自己开两个窗口对照。
落位与命名:写到项目 docs/reports/ 下(该目录通常已进 .gitignore —— 先
grep docs/reports .gitignore 确认;报告是一次性产出,不进版本控制)。目录不存在时先建。
文件名用分支维度而非日期维度:<分支简述>-审查报告.html,例如
教案对齐分支-审查报告.html。日期属于轮次,写在 section 里,不进文件名。
先 read docs/reports/ —— 已有同分支报告就追加 section,没有才新建。
样式走共享 CSS
模板在本 skill 目录内:assets/review.css(相对本 skill 目录解析)。
首次在一个仓库生成报告时,把它拷到项目 docs/reports/assets/review.css:
mkdir -p docs/reports/assets
cp <skill目录>/assets/review.css docs/reports/assets/review.css
必须拷一份而不是直接引用 skill 路径 —— 报告在 file:// 下打开,无法跨目录引用
用户主目录里的文件,且报告要能独立随项目留存。
报告用 <link rel="stylesheet" href="assets/review.css"> 外链。
新报告不重复内联样式;缺样式类就往项目侧的 CSS 里加,不在 HTML 内写 <style>。
改动若通用(新增语义类、修配色),同步回 skill 的 assets/review.css,下个仓库直接受益。
CSS 已包含:四色变量(ok/warn/bad/info)· .wrap .scores .card(.label/.grade/.delta/.note)
· .tag(t-ok/t-bad/t-warn/t-off/t-info)· .issue(p0/p1/new/fixed)
· .callout(good/note/warn)· .rounds .round-tab .evo · pre 着色类(.c/.r/.g/.y/.b)
· .g-a~.g-d 评级色 · @media print 展开全部 section ·
兼容别名段(.cards .dim .defect .why .fix .s-ok .l-ok .tbl-scroll .legacy)。
遇到用旧 class 名的既有报告:优先靠兼容别名收编,不要重排正文标记 ——
正文是审查证据,改标记有改错内容的风险,加几行 CSS 零风险。
转发例外:报告要发给他人(邮件/IM)时外链会丢失 —— 此时把 <link> 换成 <style>
并内联 CSS 全文,另存一份带 -单文件 后缀。默认仍用外链。
文档结构
header 标题 / 分支·基线·轮次数·最新 tip / 判据来源 + 刻度说明 / 当前结论
评级演进 常驻矩阵:维度 × 各轮 → 变化原因(不随标签切换)
.rounds 标签页:第 N 轮…… + 可选「需求总结(供汇报)」
section×N 每轮一个:轮次元信息 → 评分卡 → 本轮改了什么 → 判据核对
→ 变异测试 → 缺陷块 → 合并判断
footer 每轮一行:日期·tip·判据·测试数·变异结果·探针脚本路径
评级演进矩阵是多轮报告的核心价值,务必常驻在标签页外:一眼看出哪个维度升了、
为什么升。只有一轮时也放,第二轮才有对照。末行放「合并判断」的演进(不可合 → 可合)。
最新一轮默认展开(aria-selected="true",其余 hidden)。切换脚本同步
location.hash,这样能用 #round-1 直达某一轮,便于在聊天里给出链接。
判据是重建的,必须在 header 单列一行声明来源,例如「本次无需求文档,判据重建自
commit body + docstring + 新增测试断言」。读者要能一眼看出这份报告是拿什么尺子量的。
内容要点
- 实测输出原样进
<pre>,用 span 着色标出关键行 —— 这是报告可信度的来源
- 每个缺陷块配一段「为什么这是问题」,讲清影响面和根因,不止讲现象
- 评分卡
note 写扣分理由,不是形容词;复审轮的卡片加 .delta 标出与上轮的变化
- 上一轮的缺陷在新一轮要有交代:修了标
.issue.fixed,没修进「清理情况」表标遗留
- JS 只用于标签切换,十几行原生足够,不引框架
渲染必须验证(xd://browser)
const css = await tab.evaluate(() => ({
bodyBg: getComputedStyle(document.body).backgroundColor,
cardRadius: getComputedStyle(document.querySelector('.card')).borderRadius
}));
await tab.click('#tab-1');
await tab.evaluate(() => ({
visible: [...document.querySelectorAll('.round')].filter(s=>!s.hidden).map(s=>s.id),
hash: location.hash
}));
await tab.evaluate(() => document.documentElement.scrollWidth > window.innerWidth);
bodyBg 是透明说明 CSS 没加载(路径错或文件缺失)。
overflow: true 说明有元素撑破视口(通常是长 <pre> 或宽表格),要修。
每个标签页都要点一遍确认内容出得来,再滚到尾部确认 footer 正常。
反模式
- 没问就直接生成 HTML 报告 → 多做一轮可能没人要的工序,且堆无人维护的文件
- 报告写到
docs/ 根或仓库其他位置 → 该进 docs/reports/(已 gitignore);写在根下会被 git 跟踪
- 复审时新建一个文件 → 同一分支追加
<section class="round">,读者不该开两个窗口对照
- 文件名带日期或轮次(
20260810-复审-第二轮.html)→ 文件名用分支维度,日期属于轮次写在 section 里
- 报告里内联整套
<style> → 走共享 assets/review.css;缺样式类往 CSS 加
- 忘了把 skill 的
assets/review.css 拷进项目就外链 → 样式全丢(bodyBg 透明即此症状)
- 直接
<link> 指向 skill 目录里的 CSS → file:// 跨目录引用不到,报告也无法随项目留存
- 为迁就新 class 名重排既有报告的正文标记 → 正文是审查证据;靠 CSS 兼容别名收编
- 多轮报告没有评级演进矩阵 → 那是多轮报告的核心价值,藏进标签页等于没有
- 新一轮不交代上一轮的缺陷 → 修了要标
.issue.fixed,没修要在「清理情况」表标遗留
- 给完结论就收工、不提报告选项 → 用户不知道有这个选择
- 先做报告再给结论 → 判据若被纠正,报告整份重做
- 给了字母评分却没配可核查的事实依据 → 凭印象打分;说不清为什么是 A 就降一档
- 自创 A+ / F / 百分制 / 五星等刻度外的档位 → 刻度只有 A/B/C/D 四档,
± 仅表档内偏移
- 「没做错」就给 A → A 要求主动作为的证据,无缺陷的上限是 B
- 缺需求文档就直接点评代码风格 → 跳过了判据重建,产出的是偏好不是审查
- 判据是重建的却写成「不符合需求」→ 冒充了业务验收口径,必须标注来源
- 缺需求/缺 spec 就先问用户要,而不先走完 §1b 的判据阶梯 → 偷懒;8 层判据里通常有 3 层以上可用
- 仓库不用 spec 驱动却按 spec 规范扣分 → 拿别人的尺子量
- 遇到「哨兵值还是漏改」这类真歧义时自己拍板 → 结论相反的解读必须交用户
- 「这里可能有问题」而没跑一次 → 不是审查结论
- 把存量问题算进本次交付 → 失去可信度
- 批评 spec 明确声明的非目标 → 说明没读 spec
- 用
git worktree / checkout 做验证 → 动了用户工作区
- 声称「63 passed」而没自己跑一遍 → 复述而非验证
- 罗列一堆同等严重度的问题 → 用户不知道该拦哪个;必须分档
- 用户只要审查却附上修复 patch → 越界