From a4be4b5e5264e1016cec414d7d43c03c0e15c3d6 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Mon, 20 Jul 2026 23:29:22 +0800 Subject: [PATCH] docs(invariants): codify semantic review rules --- .agents/skills/dsh-code-review/SKILL.md | 5 +++-- AGENTS.md | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.agents/skills/dsh-code-review/SKILL.md b/.agents/skills/dsh-code-review/SKILL.md index 814132d481..925dc5b6ac 100644 --- a/.agents/skills/dsh-code-review/SKILL.md +++ b/.agents/skills/dsh-code-review/SKILL.md @@ -23,7 +23,8 @@ description: Use when reviewing a pull request in the deepseek-harness repo — 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 [core-data-structures](../../../docs/core-data-structures/core.md) page and any `type-equiv` entry. Internal types need no catalog entry. 4. **Registrations clean up.** Verify each new registry contribution satisfies the disposal-test contract in [packages/AGENTS.md](../../../packages/AGENTS.md). -5. **Required gates pass.** Trust the [current readiness sequence](../../../AGENTS.md#run-the-ci-gates-locally-before-marking-a-pr-ready) and `pnpm run check:pre-push` for their enforced inventory; review the semantic gaps they cannot detect. +5. **Invariant companions are semantic.** For every touched `./invariant`, require an owner event-stream or mutable-data relationship at its authoritative boundary; 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](../../../AGENTS.md#conventions); [package contract](../../../packages/AGENTS.md)). +6. **Required gates pass.** Trust the [current readiness sequence](../../../AGENTS.md#run-the-ci-gates-locally-before-marking-a-pr-ready) and `pnpm run check:pre-push` for their enforced inventory; review the semantic gaps they cannot detect. ## Manual checks @@ -38,7 +39,7 @@ description: Use when reviewing a pull request in the deepseek-harness repo — - **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 Loader export-shape failures; 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. -- **Mechanized invariants and negative controls:** trace each new or changed check through the executed top-level gate and its deliberately invalid case; confirm the real runner fails for the intended rule. +- **Invariant lifecycle and negative controls:** verify candidate observations are rejected before publication where possible, session-backed checks reconstruct durable history after late loading or HMR, and a deliberately invalid case fails through the real runner for the intended 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. diff --git a/AGENTS.md b/AGENTS.md index 860f60f1e0..fc39c69df3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -98,6 +98,7 @@ Real-API tests and demos read `DEEPSEEK_API_KEY`, optional `DEEPSEEK_BASE_URL`, - Every npm package is `@deepseek-ai/dsh-`; vendored packages keep upstream names and are `private: true`. `cordis` is a peerDependency (+ dev) of every harness package. - ESM everywhere (`"type": "module"`). Cross-package imports use package names; in-package relative imports include `.ts`. CI subprocesses that boot examples or Cordis configs run built `lib/` under plain Node; only explicit source-path regressions use tsx ([testing policy](docs/testing.md#test-subprocess-launch-modes)). - **Registrations are effects**: every contribution goes through `ctx.effect()` / `ctx.on()`; a registry's `register()` returns the disposer. +- **Runtime invariants assert owned relationships.** Check authoritative event streams or mutable data, not service or method presence, plugin metadata or effects, or fixed pure examples. If a package has no plausible relationship, an explained empty companion is correct ([package contract](packages/AGENTS.md)). - **Typed events use declaration merging** and merge-extensible maps. Event JSDoc needs `@mode` and payload `@param`; scoped keys absent from payloads need `@dshScopeScan unsupported`. Public service methods document parameters and non-void returns. - **Switch on discriminant tags.** Closed unions end in `assertNever`; merge-extensible unions fall through a documented default. - **Waterfall listeners MUST call `next()`** to delegate; returning without it is the veto ([semantics](docs/cordis-primer.md#cordis-waterfall-semantics)).