Back to Details

dsh-code-review

Compare original and translation side by side

🇺🇸

Original

English
🇨🇳

Translation

Chinese

Reviewing a DeepSeek-Harness PR

审查 DeepSeek-Harness PR

This skill is guidance, not a complete checklist. Verify and fetch the PR's live base and exact head, then run 
pnpm --silent run change-scope --base <verified-base-ref> --head <verified-head-ref>
before reading the diff and enough surrounding code to understand the design. The report identifies paths and dirty layers but does not replace semantic review. Re-establish the base and rerun it after a retarget or merge. Prioritize correctness, lifecycle, security, and broken required behavior over style; a short review with one substantiated blocker is better than a list of nits.
本 skill 是指导材料，而非完整检查清单。 先验证并获取 PR 当前的 base 和精确的 head，然后运行 
pnpm --silent run change-scope --base <verified-base-ref> --head <verified-head-ref>
，再阅读 diff 以及足够多的周边代码以理解设计。该报告会标出受影响的路径和 dirty layers，但无法取代语义审查。PR 重定目标或合并后，应重新确定 base 并再次运行。相较于样式问题，应优先关注正确性、生命周期、安全性以及必需行为失效；一份只包含一个有充分依据的阻断项的简短审查，胜过一串琐碎问题。

Sources of truth

事实来源

  • AGENTS.md and packages/AGENTS.md: standing repository and package authoring rules.
  • docs/defensive-patterns.md: subprocess, callback, async-state, and disposal bug classes.
  • docs/AGENTS.md: documentation placement and prose discipline.
  • dsh-prose-standard: required coverage and editorial judgment for comments, docs, prompts, and visible strings.
  • dsh-ci-test-reliability: isolation and regression-proof rules for resource-owning, asynchronous, or flaky tests and fixtures.
  • dsh-client-ui-ux: feedback-surface choice, overlay safety, platform window adaptation, and visual token discipline for product-user-visible GUI changes.
  • docs/testing.md and the quality-gates reference: required test tiers and gates.
  • Agent Notes: design rationale. Treat disagreement with an Agent Note as a design discussion, not an automatic veto.
  • For bilingual changes, read translation-rules.md and terminology.md; the extended translation skill is outside automatic review and runs only on explicit user invocation.
  • AGENTS.md 和 packages/AGENTS.md：仓库级与包级的长期编写规范。
  • docs/defensive-patterns.md：子进程、回调、异步状态和资源释放缺陷类别。
  • docs/AGENTS.md：文档存放位置和行文规范。
  • dsh-prose-standard：针对注释、文档、prompt 和可见字符串的必需覆盖范围与编辑判断。
  • dsh-ci-test-reliability：针对持有资源、异步或易波动的测试及 fixture 的隔离与防回归规则。
  • dsh-client-ui-ux：对于用户可见的 GUI 变更，反馈界面选择、浮层安全、平台窗口适配和视觉 token 规范。
  • docs/testing.md 和质量门禁参考：必需的测试层级与门禁。
  • Agent Notes：设计理由。与 Agent Note 存在分歧时，应将其视为设计讨论，而不是自动否决。
  • 对于双语变更，请阅读 translation-rules.md 和 terminology.md；扩展翻译 skill 不属于自动审查范围，仅在用户明确调用时运行。

Blocking requirements

阻断性要求

  1. New prose receives semantic review. Use dsh-prose-standard to critically review every added or changed Markdown passage, JSDoc, comment, prompt, description, diagnostic, and visible string. Verify required coverage, accuracy, placement, and editorial quality against the owning code or behavior; automated checks do not establish those properties.
  2. Docs match the code. Config, defaults, errors, wire fields, events, and public behavior update the package README and JSDoc in the same diff. Comments state non-obvious contracts; flag implementation narration, test walkthroughs, review history, and duplicated rationale for deletion or a link to their one home.
  3. Core type docs match. Changes to spine or seam vocabulary update the appropriate subsystems page and any 
    type-equiv
    entry. Internal types need no catalog entry.
  4. Registrations clean up. Verify each new registry contribution passes the disposal tests required by packages/AGENTS.md.
  5. Required evidence exists. Verify the author ran the relevant local checks for the diff and that CI covers the exhaustive matrix; review the semantic gaps neither can detect.
  6. Client UI copy is locale-owned. Reject product text embedded in JSX, templates, helper returns, accessibility attributes, or primitive defaults. Require typed dictionary keys, the standard 
    t
    seat or explicit localized props, 
    verify-client-ui-i18n
    , and behavior evidence in each affected locale; preserve user/model/wire data and code tokens verbatim.
  1. 新增文字必须经过语义审查。 使用 dsh-prose-standard，审慎审查每一处新增或修改的 Markdown 段落、JSDoc、注释、prompt、描述、诊断信息和可见字符串。对照所属代码或行为，核验所需覆盖是否完整、内容是否准确、位置是否恰当以及编辑质量是否达标；自动化检查无法证明这些属性成立。
  2. 文档必须与代码一致。 配置、默认值、错误、wire 字段、事件和公共行为必须在同一个 diff 中同步更新 package README 和 JSDoc。注释应说明非显而易见的契约；实现过程的流水账、测试操作说明、审查历史和重复的理由说明应删除，或改为链接到其唯一归属位置。
  3. 核心类型文档必须一致。 对 spine 或 seam 术语的改动必须更新相应的 subsystems 页面及所有相关的 
    type-equiv
    条目。内部类型无需在目录中新增条目。
  4. 注册项必须正确清理。 验证每一项新增的 registry 注册项都通过 packages/AGENTS.md 要求的 disposal 测试。
  5. 必须提供必要证据。 验证作者已针对该 diff 运行相关本地检查，并确认 CI 覆盖了完整矩阵；同时审查本地检查与 CI 均无法发现的语义缺口。
  6. 客户端 UI 文案必须由 locale 管理。 拒绝硬编码在 JSX、template、helper 返回值、无障碍属性或 primitive 默认值中的产品文案。要求使用有类型的字典键、标准 
    t
    seat 或显式本地化 props、运行 
    verify-client-ui-i18n
    ，并提供每个受影响 locale 中的行为证据；user/model/wire 数据和代码 token 必须原样保留。

Manual checks

手动检查

  • Intent and interface contracts: trace both sides of every changed interface. Confirm the implementation matches the PR and any Agent Note, including errors, cancellation, ownership, and disposal.
  • Lifecycle and concurrency: for async setup, callbacks, processes, or teardown, apply defensive-patterns.md. Check races before publication, cancellation during awaits, independent error reporting, callback containment, ownership before reentry, complete detach cleanup, and quiescent disposal.
  • Capability and consumer fit: trace every current consumer, then flag consumer-specific behavior leaking into the interface under the package rules. Flag the inverse too: a new public method on a generic service (registry, session, agent) whose only caller is one internal consumer is an unnecessary API expansion — require a private capability closure handed to that consumer at construction instead.
  • Scope, ownership, and necessity: map each abstraction, state machine, option, defensive copy, and compatibility path to its current contract, production consumer, and owning plugin or service. Challenge unrelated features and speculative generality, then test the PR against the root rules.
  • Configuration and public choices: ask what current-consumer evidence or prior art supports each default, public operation set, format, or imported external concept. Require an explicit choice or deferral when that evidence is absent.
  • Model perspective: inspect the exact prompts, tool schemas, results, and diagnostics the model receives across affected modes. Flag concepts outside the model's task, then verify stable text verbatim and dynamic behavior through snapshots or end-to-end coverage.
  • Enforcement: follow every denial path to the operation that executes it; exercise direct and alternate callers that can bypass schemas, prompts, facades, wrappers, or listener ordering.
  • Borrowed and derived state: determine whether each retained value is borrowed or owned under the package contract, then trace notifications and every cache, prompt, UI echo, replay, and query view to the documented success point and authoritative source.
  • Bounds cover the final operation: locate the owner of the complete emitted or retained result, including wrappers and metadata. Probe tiny and exact limits, oversized single chunks, and multibyte text for byte limits.
  • Real entry path: tests exercise the shipped Loader, bin, worker, ACP bridge, or subprocess where relevant. A hand-mounted plugin does not catch invalid Loader exports; a function plugin must named-export its namespace and have no default export.
  • Test strength: assertions fail on the intended regression and verify external state, logs, events, or disposal rather than restating the implementation or trusting an agent's report. Coverage is necessary but not evidence that the scenario is correct.
  • Test reliability: for a resource-owning, asynchronous, platform-sensitive, or flaky test, apply dsh-ci-test-reliability to the real worker/job topology, resource allocation, global-state restoration, synchronization, timeout budget, and quiescent teardown.
  • Negative controls: verify a deliberately invalid case fails through the real runner for each changed acceptance rule.
  • Implemented Agent Notes match shipped reality: when a PR implements a proposed Agent Note, move and rewrite it as present-tense shipped state in the same diff, then verify paths, names, and mechanisms against the implementation.
  • Transcript changes: editor-visible or model-visible changes update snapshots or explain why no snapshot applies. Review expected-output diffs as behavior changes, not formatting noise.
  • Bilingual changes: compare meaning and terminology on both sides; a green pairing hash does not prove translation quality.
  • 意图与接口契约： 追踪每个变更接口的两端。确认实现符合 PR 和所有相关 Agent Note，包括错误、取消、所有权和资源释放。
  • 生命周期与并发： 对于异步 setup、callback、process 或 teardown，应用 defensive-patterns.md。检查发布前的竞态、await 期间的取消、独立的错误报告、callback 包含关系、重入前的所有权、彻底的 detach 清理，以及达到静止状态后的资源释放。
  • 能力与消费者适配性： 追踪每个当前消费者，然后依据包级规则，指出渗入接口的消费者特定行为。反过来也必须指出：在 generic service（registry、session、agent）上新增 public method，而其唯一调用方只是内部消费者时，这是不必要的 API 扩张；应改为在构造时交给该消费者一个私有 capability 闭包。
  • 范围、所有权与必要性： 将每个抽象、状态机、选项、防御性副本和兼容路径映射到其当前契约、生产消费者以及所属 plugin 或 service。质疑无关功能和推测性泛化，然后依据根级规则检验该 PR。
  • 配置与公共选项： 询问当前消费者证据或既有实践是否支持每个默认值、公共操作集、格式或引入的外部概念。如果没有此类证据，则要求明确作出选择或暂缓处理。
  • 模型视角： 检查受影响模式下模型实际收到的确切 prompt、tool schema、结果和诊断信息。指出超出模型任务范围的概念，并通过 snapshot 或端到端覆盖验证固定文本保持逐字一致、动态行为符合预期。
  • 执行约束： 沿着每条拒绝路径追踪到实际执行相关操作的环节；测试直接调用方，以及可能绕过 schema、prompt、facade、wrapper 或监听器顺序的替代调用方。
  • 借用与派生状态： 根据 package 契约确定每个保留值是借用还是自有，然后追踪通知，以及每个 cache、prompt、UI 回显、replay 和 query view，直至文档规定的成功点和权威来源。
  • 边界覆盖最终操作： 找出完整输出或保留结果的所有者，包括 wrapper 和 metadata。测试极小值、恰好位于上限的值、超大的单个 chunk 和多字节文本，以验证字节限制。
  • 真实入口路径： 在相关场景中，测试必须走实际交付的 Loader、bin、worker、ACP bridge 或 subprocess。手动挂载的 plugin 无法发现无效的 Loader export；函数型 plugin 必须以 named export 形式导出其 namespace，且不得包含 default export。
  • 测试强度： 断言应在预期回归发生时失败，并验证外部状态、日志、事件或资源释放，而不是复述实现或相信 agent 的报告。覆盖率是必要条件，但不能证明场景正确。
  • 测试可靠性： 对于持有资源、异步、平台敏感或易波动的测试，应将 dsh-ci-test-reliability 应用于真实的 worker/job 拓扑、资源分配、全局状态还原、同步、超时预算和静止状态下的 teardown。
  • 负向对照： 对于每项发生变更的验收规则，验证一个刻意构造的无效案例会通过真实 runner 失败。
  • 已实现的 Agent Notes 必须与实际交付状态一致： 当 PR 实现某项拟议的 Agent Note 时，在同一个 diff 中将其移动并改写为以现在时描述的已交付状态，然后对照实现验证路径、名称和机制。
  • Transcript 变更： 对编辑器或模型可见的改动，应更新 snapshot，或说明为何不适用 snapshot。应将预期输出的差异视为行为变更，而不是格式噪音。
  • 双语变更： 比较两种语言在含义和术语上的一致性；配对 hash 校验通过并不能证明翻译质量。

Reporting findings

报告问题

State the defect, location, impact, and evidence. Place a localized defect inline on the tightest relevant diff range; use a PR-level comment for cross-cutting architecture, scope, or review-wide synthesis. Separate blockers from suggestions and omit issues already enforced by a green gate. Use the existing GitHub review thread for replies. When receiving review, verify each claim and fix or rebut it on technical grounds without performative agreement.
说明缺陷、位置、影响和证据。将发现的问题精确放在最紧密相关的 diff 范围内；跨切面的架构、范围或全局审查综合意见应使用 PR 级评论。将阻断项与建议分开，并省略那些已由通过的门禁覆盖的问题。回复应使用现有的 GitHub review thread。收到审查意见时，应核实每项主张，并基于技术依据修复或反驳，不要表演式赞同。