dsh-code-review

Reviewing a 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.

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.
• docs/testing.md and the quality-gates Agent Note: 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.

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. Invariant companions are semantic. For every touched ./invariant, require an owner event-stream or mutable-data relationship at the point where that package can observe it; service or method presence, plugin metadata or effects, and fixed pure examples belong in type, load, or unit tests. Accept an empty installer when its package-specific reason establishes that no plausible runtime relationship exists; do not demand an invented check merely to eliminate emptiness (repository rule; package invariant rules).
6. 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.
7. 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.