企业官网建设流程全解析

Sanity 代码评审质量门禁：五轴评审方法、变更规模管控与人机协作的多模型评审实践

【免费下载链接】sanitySanity Studio – Rapidly configure content workspaces powered by structured content项目地址: https://gitcode.com/GitHub_Trending/sa/sanity

本文以 Sanity 仓库中的 code-review-and-quality 技能文档 为主线，完整讲解其"所有变更合并前必须评审"的质量门禁体系：覆盖正确性、可读性、架构、安全、性能五个评审轴的判定标准，变更规模（100/300/1000 行）管控与拆分策略，以及多模型协作评审流程；并结合仓库中 CI 检查、pre-commit 钩子、死代码检测等真实工程设施佐证，读完后你能在一套 pnpm monorepo 中落地可复现的代码评审质量门禁。

一、质量门禁心智：每个变更都要过评审

技能文档开宗明义：多维度评审 + 质量门禁，每个变更在合并前都要被评审，没有例外。评审覆盖五个轴：正确性（Correctness）、可读性（Readability）、架构（Architecture）、安全（Security）、性能（Performance）。

文档同时给出了明确的批准标准（approval standard），这是整套方法中最容易被忽视、却最实际的一条：

当一个变更确定能改善整体代码健康度时就应当批准，哪怕它并不完美。完美的代码不存在——目标是持续改进。不要因为"它不是我会写的方式"就阻塞变更；只要它改善了代码库并遵循项目约定，就批准它。

适用场景（文档 "When to Use" 完整清单）：

• 合并任何 PR 或变更之前
• 完成一项功能实现之后
• 需要评估另一个 agent 或模型产出的代码时
• 重构既有代码时
• 任何 bug 修复之后（要同时评审修复本身和回归测试）

从仓库结构看，这份文档位于.agents/skills/目录，与 performance-optimization、code-simplification、pr-description 等技能并列，是 Sanity 这类"人 + AI Agent 共同提交代码"的 monorepo 里，给所有评审者（包括 agent）使用的统一评审手册。

二、五轴评审模型

2.1 正确性（Correctness）

核心问题：代码是否做了它声称要做的事？

• 是否匹配规格或任务要求？
• 边界情况是否处理（null、空值、边界值）？
• 错误路径是否处理（而不只是 happy path）？
• 是否通过所有测试？这些测试是否真的在测对的东西？
• 是否存在差一错误（off-by-one）、竞态条件或状态不一致？

Sanity 仓库为这一轴提供了可执行的验证基线：根 vitest.config.mts 注册了 20 多个测试 project（@sanity/schema、@sanity/mutator、packages/sanity、perf/bench、e2e等），其中注释特别说明perf/bench的 mock 契约测试"是 bench mock 的漂移探测器，必须在每个 PR 上运行，而不只是 label 门控的 bench 运行"。AGENTS.md 也给出了标准验证路径：pnpm build && pnpm test无需任何认证即可覆盖绝大多数代码变更的正确性验证，组件级测试则用createMockAuthStore避免真实认证依赖。

2.2 可读性与简洁性（Readability & Simplicity）

核心问题：另一位工程师（或 agent）不需要作者解释，就能理解这段代码吗？

• 命名是否具有描述性、与项目约定一致？（不要出现无上下文的temp、data、result）
• 控制流是否直接（避免嵌套三元、深层回调）？
• 代码是否按逻辑组织（相关代码聚集、模块边界清晰）？
• 是否存在应当简化的"炫技"写法？
• 能否用更少的行数完成？（1000 行能用 100 行解决就是失败）
• 抽象是否配得上它的复杂度？（不要等到第三个用例才泛化）
• 注释是否有助于说明非显而易见的意图？（但不要给显然的代码加注释）
• 是否存在死代码残留：no-op 变量（_unused）、向后兼容垫片、// removed注释？

这一轴在 Sanity 仓库中有自动化兜底：lefthook.yml 在每次 commit 时对暂存文件并行运行oxfmt（格式化）和oxlint --fix（含类型感知的 Rust 静态检查），把"命名/格式/明显坏味道"这类最低限度的可读性问题拦在提交之前，评审者就可以把注意力留给更高层次的结构性问题。

2.3 架构（Architecture）

核心问题：这个变更是否契合系统的设计？

• 它遵循既有模式，还是引入了新模式？如果是新模式，理由是否充分？
• 是否保持了干净的模块边界？
• 是否有应当共享的代码重复？
• 依赖方向是否正确（无循环依赖）？
• 抽象层次是否恰当（既不过度设计，也不过度耦合）？

仓库层面，模块边界是显式治理的：根 package.json 的 devDependencies 中引入了eslint-plugin-boundaries，knip.jsonc 的注释进一步确认它通过 oxlint 的jsPlugins以import/resolver设置生效。也就是说，"依赖是否流向正确方向"这类架构轴问题，在 Sanity 中有一部分是靠 lint 规则机器强制的，评审时重点关注的是规则覆盖不到的结构性决策。

2.4 安全（Security）

核心问题：这个变更是否引入了漏洞？

• 用户输入是否经过验证和净化？
• 密钥是否远离代码、日志和版本库？
• 需要鉴权的地方是否检查了认证/授权？
• SQL 查询是否参数化（禁止字符串拼接）？
• 输出是否经过编码以防 XSS？
• 依赖是否来自可信源且无已知漏洞？
• 外部来源的数据（API、日志、用户内容、配置文件）是否被视为不可信？
• 外部数据流在进入逻辑或渲染前，是否在系统边界处完成验证？

文档的 "See Also" 指向了security-and-hardening技能与references/security-checklist.md，但从仓库实际结构看，该参考文件当前并不存在（仓库内检索*checklist*无结果）——可以推断这类细则文档尚未随技能入库。就当前仓库而言，安全轴最具体、可验证的落地是CI workflow 本身的安全审计：zizmor.yml 配置了 zizmor 规则，对所有actions/*、pnpm/*、sanity-io/*等第三方 GitHub Action 强制ref-pin，对未列出的其余 Action 要求hash-pin（内容哈希锁定），根 package.json 中的lint:workflows脚本执行该审计并在 high 级别发现时让 CI 失败。这正是文档所说"安全敏感变更需要安全视角的评审"在本仓库的实际形态。

2.5 性能（Performance）

核心问题：这个变更是否引入了性能问题？

• 是否存在 N+1 查询模式？
• 是否存在无界循环或不受约束的数据抓取？
• 是否本应异步的操作是同步的？
• UI 组件是否有不必要的重渲染？
• 列表端点是否缺少分页？
• 热路径中是否创建了大对象？

Sanity 仓库对这一轴的回答是一套独立基准设施：AGENTS.md 记录了perf/bench套件——它针对本地 mock 的 Sanity API基准测试已构建的 studio，完全隔离、无 token、无网络。评审涉及性能敏感的改动时，可以要求作者提供基准对比：

pnpm build:bench # 先构建包 + bench studio pnpm bench run --scenario singleString # 绝对交互基准 pnpm bench run --mode pageload --scenario singleString # 加载性能 + 包体积 pnpm bench:unit # mock 契约 + 统计单元测试

这对应文档"诚实评审"一节的要求：尽可能量化问题——"这个 N+1 查询会给列表里每个条目增加约 50ms"优于"这里可能有点慢"。基准数据就是这种量化说法的证据来源。

三、变更规模管控：100 / 300 / 1000 行

小而聚焦的变更更易评审、合并更快、部署更安全。文档给出的目标规模：

~100 行变更 → 良好。一次评审就能看完。 ~300 行变更 → 如果是单一逻辑变更，可接受。 ~1000 行变更 → 太大。拆掉它。

什么算"一个变更"：一个自包含的修改，解决一件事，包含相关测试，提交后系统仍保持可用。它是一个特性的一部分——而不是整个特性。

变更过大时的拆分策略：

何时可以接受大变更：整文件删除，以及评审者只需验证意图、无需逐行检查的自动化重构。

文档还有一条与"架构轴"直接呼应的铁律：把重构与特性开发分开。一个既重构既有代码又添加新行为的变更是两个变更——分开提交。小的清理（变量重命名）可以由评审者酌情允许一并提交。

四、变更描述：让描述在版本历史中独立成立

每个变更都需要一段能在版本库历史中独立成立的描述。

• 首行：短、祈使句、独立可读。写"Delete the FizzBuzz RPC"而不是"Deleting the FizzBuzz RPC"。必须信息量足够，让搜索历史的人不读 diff 也能理解变更。
• 正文：改了什么、为什么改。包含上下文、决策与代码中看不到的推理。相关时链接 bug 编号、基准结果或设计文档。当方法存在不足时，明确承认。
• 反模式："Fix bug"、"Fix build"、"Add patch"、"Moving code from A to B"、"Phase 1"、"Add convenience functions"。

这条规范在 Sanity 仓库中是被 CI 硬性执行的：AGENTS.md 要求 PR 标题遵循conventional commits格式type(scope): 小写描述（type 限于feat/fix/chore/docs/refactor/test/perf/ci，scope 必填），不合规的标题直接让 CI 失败。例如fix(groq): resolve CJS type export issue是合格样例，而Fix(cli): Handle missing config（type 与描述未小写）或added new feature（缺 type/scope）都不合格。也就是说，文档中"首行要信息量足够、可独立检索"的评审标准，在 Sanity 里被前置成了提交前的格式门禁。

五、五步评审流程

5.1 第一步：理解上下文

在看代码之前先理解意图：

- 这个变更想达成什么？ - 它实现的是哪个规格或任务？ - 预期的行为变化是什么？

5.2 第二步：先评审测试

测试揭示意图与覆盖范围：

- 这个变更有测试吗？ - 测的是行为（而非实现细节）吗？ - 边界情况覆盖了吗？ - 测试命名有描述性吗？ - 如果代码变了，这些测试能捕获回归吗？

5.3 第三步：评审实现

带着五个轴遍历代码，对每个被修改的文件依次问：

1. 正确性：这段代码做的是测试所声称的事吗？ 2. 可读性：我不借助解释能看懂吗？ 3. 架构：它契合系统吗？ 4. 安全：有漏洞吗？ 5. 性能：有瓶颈吗？

5.4 第四步：给发现分级

为每条评论标注严重级别，让作者分清"必须改"和"可选"：

文档强调这一设计的动机：防止作者把所有反馈都当成强制项，在可选建议上浪费时间。

5.5 第五步：验证"验证本身"

检查作者的验证故事（verification story）：

- 跑了哪些测试？ - 构建通过了吗？ - 变更经过手动测试了吗？ - UI 变更有截图吗？ - 有 before/after 对比吗？

在 Sanity 仓库，"验证本身"有客观的机器判据。AGENTS.md 列出了每个 PR 都必须通过的 CI 检查：

评审者在"验证验证"这一步可以直接核对：作者声称的"测试通过、构建成功"是否对应到上面这张表，而不是停留在口头保证。

六、多模型评审：让不同模型承担不同视角

文档提出用不同模型覆盖不同评审视角，以规避单一模型的盲区：

模型 A 写代码 │ ▼ 模型 B 评审正确性与架构 │ ▼ 模型 A 处理反馈 │ ▼ 人类做最终裁决

其原理是：不同模型有不同的盲区，交叉评审能发现单一模型会漏掉的问题。文档给出的评审 agent 提示词模板：

Review this code change for correctness, security, and adherence to our project conventions. The spec says [X]. The change should [Y]. Flag any issues as Critical, Important, or Suggestion.

这个"模型写码 → 模型互审 → 人类终裁"的流水线，在 Sanity 仓库中有一套与 AGENTS.md 完全对应的落地流程：

1. Agent 先建 Draft PR，标题必须符合 conventional commits，并强制打上🤖 bot标签以便团队识别 agent 来源的 PR；
2. Prompter（提出需求的人）先审 draft，相当于"模型 B 之外的人类第二视角"；
3. Prompter 批准且 CI 全绿后执行gh pr ready转正式评审；
4. 团队评审并合并。

再叠加 CONTRIBUTING.md 中的人类协作规范——PR 应"随时可合并"（自评过、linted、测试套件通过），合并进main需要至少两位评审者批准，且优先 squash + merge——整套流程恰好就是"多模型/多视角评审 + 人类最终裁决"文档化之后的工程实现。

七、死代码卫生：显式列出，确认后再删

在任何重构或实现变更之后，检查被孤立的代码：

1. 找出现在不可达或未被使用的代码；
2. 显式列出来；
3. 删除前先问："以下这些现在没用的元素，要我现在移除吗：[列表]？"

原则是：别把死代码留着不管——它会迷惑未来的读者和 agent；但也不要悄悄删除自己不确定该不该删的东西，拿不准就问。文档给出的输出格式示例：

DEAD CODE IDENTIFIED: - formatLegacyDate() in src/utils/date.ts — 被 formatDate() 取代 - OldTaskCard component in src/components/ — 被 TaskCard 取代 - LEGACY_API_URL constant in src/config.ts — 无剩余引用 → 这些可以安全移除吗？

Sanity 仓库把这件事做成了工具化流程：根 package.json 的check:deps即knip --dependencies，配合 knip.jsonc 中按 workspace 逐一声明的entry/project配置（例如packages/sanity的入口是src/_exports/*.ts与bin/sanity），让"未使用代码与依赖"在 CI 中可机器检测。更值得玩味的是配置本身的注释风格——"!src/**/__test{,s}__"旁写着 "These files might actually be safe to delete?"，scripts的 entry 旁写着 "Seems genuinely unused, but important?"——这正是"列出 → 标注存疑 → 交给判断"的死代码卫生文化在配置文件里的微观体现。

八、评审速度与分歧处理

评审速度。慢评审阻塞的是整个团队——切换到评审模式的成本，小于强加给别人的等待成本：

• 一个工作日内响应——这是上限，不是目标；
• 理想节奏：评审请求到达后尽快响应，除非正深度沉浸在某段编码中。典型变更应能在一天内完成多轮评审；
• 优先保证单次响应的速度，而不是尽快给出最终批准。快速反馈即使需要多轮也能减少挫败感；
• 超大变更：请作者拆分，而不是硬啃一个巨型变更集。

分歧处理层级（解决评审争议时自上而下适用）：

1. 技术事实与数据优先于观点与偏好；
2. 风格指南是风格问题上的绝对权威；
3. 软件设计必须用工程原则评估，而不是个人偏好；
4. 代码库一致性可以接受，前提是不损害整体健康度。

文档还特别强调一条反直觉但重要的规则：不要接受"我之后会清理"。经验表明延后清理几乎不会发生。除非是真正的紧急情况，要求清理在提交前完成；若本次变更无法顺手处理周边问题，就要求开一个 bug 并指派给提交者本人。

九、诚实评审

无论是评审自己写的、另一个 agent 写的，还是人类写的代码，文档给出了五条诚实准则：

• 不要橡皮图章。没有评审证据的 "LGTM" 对谁都没有帮助。
• 不要粉饰真实问题。一个会打到生产环境的 bug，说成"这可能是个小问题"是不诚实的。
• 尽可能量化问题。"这个 N+1 查询会给列表里每个条目增加约 50ms" 优于 "这里可能有点慢"。
• 对有明显问题的方案要顶回去。阿谀奉承是评审的失效模式。实现有问题就直说，并提出替代方案。
• 优雅地接受否决。如果作者掌握完整上下文且不同意，服从他的判断。评论针对代码而不是人——把对人的批评重构为对代码的批评。

十、依赖纪律

代码评审的一部分是依赖评审。文档要求在添加任何依赖之前问五个问题：

1. 现有技术栈能解决这个问题吗？（通常可以。）
2. 这个依赖有多大？（检查包体积影响。）
3. 它是否被积极维护？（看最近提交、开放 issue。）
4. 它有已知漏洞吗？（npm audit）
5. 它的许可证是什么？（必须与项目兼容。）

规则：优先使用标准库和既有工具，而不是新依赖。每个依赖都是一笔负债。

Sanity 仓库对这条纪律的执行相当严格，多处可见证据：

• 强制 pnpm：根package.json依赖only-allow，knip.jsonc 的注释提到preinstall脚本中的npx only-allow pnpm，杜绝包管理器混用；
• 新发布依赖的"年龄门槛"：工作区配置了minimumReleaseAge: 1440（1 天），且拒绝已锁定的同名新版本低于该年龄的包，防止"刚发布就有漏洞"的依赖被秒装；确有必要的包需显式加入minimumReleaseAgeExclude并附简短注释（见 AGENTS.md 的依赖章节）；
• 未使用依赖常态化检查：check:deps（knip）是每个 PR 的 CI 必过项，"加了又没用"的依赖活不过合并。

十一、评审清单（Review Checklist）

文档最后提供了一份可直接套用的完整清单，此处完整保留：

Review: [PR/变更标题]

Context（上下文）

• 我理解这个变更做了什么、为什么做

Correctness（正确性）

• 变更匹配规格/任务要求
• 边界情况已处理
• 错误路径已处理
• 测试充分覆盖了该变更

Readability（可读性）

• 命名清晰且一致
• 逻辑直接
• 无不必要的复杂度

Architecture（架构）

• 遵循既有模式
• 无不必要的耦合或依赖
• 抽象层次恰当

Security（安全）

• 代码中没有密钥
• 输入在边界处经过验证
• 无注入类漏洞
• 鉴权检查就位
• 外部数据源被当作不可信

Performance（性能）

• 无 N+1 模式
• 无无界操作
• 列表端点有分页

Verification（验证）

• 测试通过
• 构建成功
• 手动验证已完成（如适用）

Verdict（结论）

• Approve— 可合并
• Request changes— 问题必须被处理

十二、常见借口与危险信号

文档把评审中常见的自我合理化集中列成对照表：

对应的**危险信号（Red Flags）**清单：

• 没有任何评审就被合并的 PR
• 只检查测试是否通过的评审（忽略其他轴）
• 没有实际评审证据的 "LGTM"
• 安全敏感变更没有安全视角的评审
• "太大没法好好评审"的巨型 PR（拆掉它）
• bug 修复 PR 没有回归测试
• 没有严重级别标注的评审评论——无法区分必须改与可选
• 接受"我之后修"——它永远不会发生

十三、收尾验证：评审完成后的检查点

评审流程走完后的最终确认清单：

• 所有 Critical 问题已解决
• 所有 Important 问题已解决，或被显式推迟并附理由
• 测试通过
• 构建成功
• 验证故事已被记录（改了什么、如何验证的）

落到 Sanity 仓库的日常操作上，这份收尾清单对应的就是可重复执行的命令序列：pnpm lint:fix（格式 + 自动修复）→pnpm build→pnpm test（快照变化时用pnpm test -- -u更新并复核 diff）→pnpm depcheck（依赖纪律）→pnpm lint:workflows（workflow 安全）。把这些命令的输出作为"验证故事"附在 PR 描述中，评审者在第五步"验证验证"时即可逐条核对——这正是本文开篇那条批准标准的可操作形态：用证据判断变更是否改善了代码库，而非凭印象放行。

【免费下载链接】sanitySanity Studio – Rapidly configure content workspaces powered by structured content项目地址: https://gitcode.com/GitHub_Trending/sa/sanity