Sanity 代码评审质量门禁:五轴评审方法、变更规模管控与人机协作的多模型评审实践 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 多个测试 projectsanity/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-boundariesknip.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核心问题这个变更是否引入了性能问题是否存在 N1 查询模式是否存在无界循环或不受约束的数据抓取是否本应异步的操作是同步的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 契约 统计单元测试这对应文档诚实评审一节的要求尽可能量化问题——这个 N1 查询会给列表里每个条目增加约 50ms优于这里可能有点慢。基准数据就是这种量化说法的证据来源。三、变更规模管控100 / 300 / 1000 行小而聚焦的变更更易评审、合并更快、部署更安全。文档给出的目标规模~100 行变更 → 良好。一次评审就能看完。 ~300 行变更 → 如果是单一逻辑变更可接受。 ~1000 行变更 → 太大。拆掉它。什么算一个变更一个自包含的修改解决一件事包含相关测试提交后系统仍保持可用。它是一个特性的一部分——而不是整个特性。变更过大时的拆分策略策略做法适用场景Stack堆叠提交一个小变更基于它开始下一个顺序依赖By file group按文件组给需要不同评审者的文件组各开一个变更横切关注点Horizontal横向先创建共享代码/桩再创建消费方分层架构Vertical纵向把特性拆成更小的全栈切片特性开发何时可以接受大变更整文件删除以及评审者只需验证意图、无需逐行检查的自动化重构。文档还有一条与架构轴直接呼应的铁律把重构与特性开发分开。一个既重构既有代码又添加新行为的变更是两个变更——分开提交。小的清理变量重命名可以由评审者酌情允许一并提交。四、变更描述让描述在版本历史中独立成立每个变更都需要一段能在版本库历史中独立成立的描述。首行短、祈使句、独立可读。写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/ciscope 必填不合规的标题直接让 CI 失败。例如fix(groq): resolve CJS type export issue是合格样例而Fix(cli): Handle missing configtype 与描述未小写或added new feature缺 type/scope都不合格。也就是说文档中首行要信息量足够、可独立检索的评审标准在 Sanity 里被前置成了提交前的格式门禁。五、五步评审流程5.1 第一步理解上下文在看代码之前先理解意图- 这个变更想达成什么 - 它实现的是哪个规格或任务 - 预期的行为变化是什么5.2 第二步先评审测试测试揭示意图与覆盖范围- 这个变更有测试吗 - 测的是行为而非实现细节吗 - 边界情况覆盖了吗 - 测试命名有描述性吗 - 如果代码变了这些测试能捕获回归吗5.3 第三步评审实现带着五个轴遍历代码对每个被修改的文件依次问1. 正确性这段代码做的是测试所声称的事吗 2. 可读性我不借助解释能看懂吗 3. 架构它契合系统吗 4. 安全有漏洞吗 5. 性能有瓶颈吗5.4 第四步给发现分级为每条评论标注严重级别让作者分清必须改和可选前缀含义作者应采取的行动无前缀必须修改合并前必须处理Critical:阻塞合并安全漏洞、数据丢失、功能损坏Nit:次要、可选作者可忽略——格式、风格偏好Optional:/Consider:建议值得考虑但不强制FYI仅提供信息无需行动——供将来参考文档强调这一设计的动机防止作者把所有反馈都当成强制项在可选建议上浪费时间。5.5 第五步验证验证本身检查作者的验证故事verification story- 跑了哪些测试 - 构建通过了吗 - 变更经过手动测试了吗 - UI 变更有截图吗 - 有 before/after 对比吗在 Sanity 仓库验证本身有客观的机器判据。AGENTS.md 列出了每个 PR 都必须通过的 CI 检查检查命令说明Formatpnpm check:format基于 oxfmt用pnpm chore:format:fix修复Oxlintpnpm check:oxlintRust linter含类型感知规则与 TypeScript 类型检查Unit Testspnpm testVitestCI 中分片运行Export Testspnpm test:exports确保 ESM/CJS/DTS 均可用Dep Checkpnpm depcheck找出未使用/缺失的依赖Zizmorpnpm lint:workflows审计 workflow 安全问题high 级别发现即失败PR TitleConventional commits如feat(scope): description评审者在验证验证这一步可以直接核对作者声称的测试通过、构建成功是否对应到上面这张表而不是停留在口头保证。六、多模型评审让不同模型承担不同视角文档提出用不同模型覆盖不同评审视角以规避单一模型的盲区模型 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 完全对应的落地流程Agent 先建 Draft PR标题必须符合 conventional commits并强制打上 bot标签以便团队识别 agent 来源的 PRPrompter提出需求的人先审 draft相当于模型 B 之外的人类第二视角Prompter 批准且 CI 全绿后执行gh pr ready转正式评审团队评审并合并。再叠加 CONTRIBUTING.md 中的人类协作规范——PR 应随时可合并自评过、linted、测试套件通过合并进main需要至少两位评审者批准且优先 squash merge——整套流程恰好就是多模型/多视角评审 人类最终裁决文档化之后的工程实现。七、死代码卫生显式列出确认后再删在任何重构或实现变更之后检查被孤立的代码找出现在不可达或未被使用的代码显式列出来删除前先问以下这些现在没用的元素要我现在移除吗[列表]原则是别把死代码留着不管——它会迷惑未来的读者和 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?——这正是列出 → 标注存疑 → 交给判断的死代码卫生文化在配置文件里的微观体现。八、评审速度与分歧处理评审速度。慢评审阻塞的是整个团队——切换到评审模式的成本小于强加给别人的等待成本一个工作日内响应——这是上限不是目标理想节奏评审请求到达后尽快响应除非正深度沉浸在某段编码中。典型变更应能在一天内完成多轮评审优先保证单次响应的速度而不是尽快给出最终批准。快速反馈即使需要多轮也能减少挫败感超大变更请作者拆分而不是硬啃一个巨型变更集。分歧处理层级解决评审争议时自上而下适用技术事实与数据优先于观点与偏好风格指南是风格问题上的绝对权威软件设计必须用工程原则评估而不是个人偏好代码库一致性可以接受前提是不损害整体健康度。文档还特别强调一条反直觉但重要的规则不要接受我之后会清理。经验表明延后清理几乎不会发生。除非是真正的紧急情况要求清理在提交前完成若本次变更无法顺手处理周边问题就要求开一个 bug 并指派给提交者本人。九、诚实评审无论是评审自己写的、另一个 agent 写的还是人类写的代码文档给出了五条诚实准则不要橡皮图章。没有评审证据的 LGTM 对谁都没有帮助。不要粉饰真实问题。一个会打到生产环境的 bug说成这可能是个小问题是不诚实的。尽可能量化问题。这个 N1 查询会给列表里每个条目增加约 50ms 优于 这里可能有点慢。对有明显问题的方案要顶回去。阿谀奉承是评审的失效模式。实现有问题就直说并提出替代方案。优雅地接受否决。如果作者掌握完整上下文且不同意服从他的判断。评论针对代码而不是人——把对人的批评重构为对代码的批评。十、依赖纪律代码评审的一部分是依赖评审。文档要求在添加任何依赖之前问五个问题现有技术栈能解决这个问题吗通常可以。这个依赖有多大检查包体积影响。它是否被积极维护看最近提交、开放 issue。它有已知漏洞吗npm audit它的许可证是什么必须与项目兼容。规则优先使用标准库和既有工具而不是新依赖。每个依赖都是一笔负债。Sanity 仓库对这条纪律的执行相当严格多处可见证据强制 pnpm根package.json依赖only-allowknip.jsonc 的注释提到preinstall脚本中的npx only-allow pnpm杜绝包管理器混用新发布依赖的年龄门槛工作区配置了minimumReleaseAge: 14401 天且拒绝已锁定的同名新版本低于该年龄的包防止刚发布就有漏洞的依赖被秒装确有必要的包需显式加入minimumReleaseAgeExclude并附简短注释见 AGENTS.md 的依赖章节未使用依赖常态化检查check:depsknip是每个 PR 的 CI 必过项加了又没用的依赖活不过合并。十一、评审清单Review Checklist文档最后提供了一份可直接套用的完整清单此处完整保留Review: [PR/变更标题]Context上下文我理解这个变更做了什么、为什么做Correctness正确性变更匹配规格/任务要求边界情况已处理错误路径已处理测试充分覆盖了该变更Readability可读性命名清晰且一致逻辑直接无不必要的复杂度Architecture架构遵循既有模式无不必要的耦合或依赖抽象层次恰当Security安全代码中没有密钥输入在边界处经过验证无注入类漏洞鉴权检查就位外部数据源被当作不可信Performance性能无 N1 模式无无界操作列表端点有分页Verification验证测试通过构建成功手动验证已完成如适用Verdict结论Approve— 可合并Request changes— 问题必须被处理十二、常见借口与危险信号文档把评审中常见的自我合理化集中列成对照表借口现实能跑就行可读性差、不安全或架构错误的可运行代码会制造复利式累积的债务。我写的我知道它是对的作者对自己写下的假设是盲的。每个变更都受益于另一双眼睛。之后会清理的之后永远不会来。评审就是质量门禁——用它。要求在合并前而非合并后清理。AI 生成的代码应该没问题AI 代码需要更多审视而不是更少。它自信且看起来合理即使它是错的。测试通过了所以没问题测试是必要的但不充分。它抓不到架构问题、安全问题或可读性问题。对应的**危险信号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:workflowsworkflow 安全。把这些命令的输出作为验证故事附在 PR 描述中评审者在第五步验证验证时即可逐条核对——这正是本文开篇那条批准标准的可操作形态用证据判断变更是否改善了代码库而非凭印象放行。【免费下载链接】sanitySanity Studio – Rapidly configure content workspaces powered by structured content项目地址: https://gitcode.com/GitHub_Trending/sa/sanity创作声明:本文部分内容由AI辅助生成(AIGC),仅供参考