From 329e5c3e2e90aaf4538c71bc3fd176ded70eaa24 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 20 Jun 2026 13:51:09 +0800 Subject: [PATCH 1/4] docs(session-persistence): state coordinator-contract role as current fact (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reword the suite's module doc to describe what it IS — each scenario lives here once and runs per backend through the fixture — rather than narrating that the scenarios were previously duplicated in the per-backend specs. Per the repo doc-current-state convention (no process/history in comments). --- packages/session-persistence/tests/coordinator-contract.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/session-persistence/tests/coordinator-contract.ts b/packages/session-persistence/tests/coordinator-contract.ts index 7769e6f5ec..9f42eebd10 100644 --- a/packages/session-persistence/tests/coordinator-contract.ts +++ b/packages/session-persistence/tests/coordinator-contract.ts @@ -20,9 +20,8 @@ * write path — never the storage primitives directly — so it runs unchanged for * every backend (memory / jsonl / sqlite). * - * Each scenario here was previously DUPLICATED in `jsonl.spec.ts` and - * `sqlite.spec.ts`; it now lives once and runs once per backend through the - * fixture. The per-backend specs keep ONLY their storage-mechanics tests. + * Each scenario lives here once and runs once per backend through the fixture; + * the per-backend specs keep ONLY their storage-mechanics tests. * * @module @deepseek-ai/dsh-session-persistence/tests/coordinator-contract */ From 16304872e134a4e1f712973c7d5d3c0624c1ff4e Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 20 Jun 2026 13:51:30 +0800 Subject: [PATCH 2/4] docs(agent-loop): drop PR-letter ref from cancel test header (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cancel.spec.ts module doc named "PR C", narrating the change's origin — process/history a reader of the current test does not need. Per the repo doc-current-state convention, describe only what the suite tests. --- packages/agent-loop/tests/cancel.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/agent-loop/tests/cancel.spec.ts b/packages/agent-loop/tests/cancel.spec.ts index 9392b4b3d0..a4e9e13a1c 100644 --- a/packages/agent-loop/tests/cancel.spec.ts +++ b/packages/agent-loop/tests/cancel.spec.ts @@ -1,5 +1,5 @@ /** - * Tests for the queue-aware `Agent.cancel()` primitive (PR C). `cancel()` is the + * Tests for the queue-aware `Agent.cancel()` primitive. `cancel()` is the * broad verb — it clears queued + steering work, aborts an in-flight step, and * drops a turn about to start — whereas `abort()` kills only the current step. * These tests exercise every window where a cancel can land (idle, pre-step, From 44762efbd7e387201c52fef78260225b77907f7e Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 20 Jun 2026 13:51:42 +0800 Subject: [PATCH 3/4] docs(acp): drop PR-letter ref from dispose test comment (review) The disconnect-mid-prompt test comment said "PR D's per-agent AgentHandle teardown", narrating the change's origin. Per the repo doc-current-state convention, state the mechanism (the session's AgentHandle teardown) without naming the PR that introduced it. --- packages/acp/tests/dispose.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/acp/tests/dispose.spec.ts b/packages/acp/tests/dispose.spec.ts index 3196994a0e..dd27cc7e34 100644 --- a/packages/acp/tests/dispose.spec.ts +++ b/packages/acp/tests/dispose.spec.ts @@ -85,7 +85,7 @@ describe('acp bridge — disposal & HMR safety', () => { it('a client disconnect mid-prompt disposes the session (no registered agent left)', async () => { // The ACP transport closes (editor quits) while a turn runs. The bridge must - // settle the in-flight prompt cancelled and DISPOSE the agent (PR D's + // settle the in-flight prompt cancelled and DISPOSE the agent (the session's // per-agent AgentHandle teardown) rather than leaving an orphaned running — // or even idled-but-still-registered — agent whose updates are swallowed. const harness = await makeBridgeHarness({ storageDir, script: ['hang'] }) From 38cc62b6445b7941a7c1b4d5aa65f8c59356b82b Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 20 Jun 2026 13:53:56 +0800 Subject: [PATCH 4/4] docs(AGENTS): forbid naming the change unit (PR/commit) in comments & test names (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The doc-current-state convention already forbade process-narration phrasing, but review caught a subtler slip it did not name explicitly: comments and test descriptions that reference the PR / stack position that introduced the code ("(PR D's teardown)", "Tests for … (PR C)", "identity no longer matters"). The reader of the current tree has no PR D or prior design to anchor against. Add an explicit clause: never name the unseeable change unit in a comment, JSDoc, or test name — state the mechanism instead. --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 9c584b2072..424780bf74 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -176,7 +176,7 @@ In the **core** packages (`packages/llm`, `packages/tools`, `packages/agent`, `p Verbose documentation is fine **as long as docs and code stay strictly in sync**. Out-of-sync docs are worse than no docs. **When you change code, update its docs in the SAME change** — grep the package README and the module/JSDoc comments for the old behavior (config keys, defaults, error codes, wire field names, event names) and fix every hit. CI runs `pnpm run doc-sync` (`doc-typecheck` + `verify-event-taxonomy` + `verify-md-wrap` + `verify-md-links`), which typechecks every fenced `ts` block in `README.md`, `docs/**/*.md`, and `packages/*/README.md`, verifies the event-taxonomy table against source, asserts no hard-wrapped prose paragraphs, and checks that every relative Markdown cross-link resolves — across those files plus `AGENTS.md` / `packages/AGENTS.md` — but that scope does NOT catch prose drift in `AGENTS.md` / `packages/AGENTS.md` / `packages/README.md` (config keys, defaults, error codes), so keeping those in sync remains on the author. Every module has a module-level doc comment explaining its role. Every exported class, interface, type, function, and non-obvious method has a JSDoc that explains semantics (not just the name) — contracts (what events fire when), disposal behavior, error behavior, and extension intent. Internal helpers get docs only where non-obvious. Prefer one-liners when one line suffices. -**Document the CURRENT state — the "what" and "why" — never the PROCESS or HISTORY of how it got there.** A comment, JSDoc, or doc paragraph describes what the code *is* and why it is that way, as if it had always been so. Do NOT narrate the change that produced it: no "previously X, now Y", "changed from", "used to", "this replaces", "the old map", "renamed", "moved here", "as of this PR", or "(was …)". Such phrasing rots the instant the next change lands, and a reader of the current code does not need the diff narrated in prose — that belongs in the commit message, the PR description, or an RFC (the durable home for "why we moved away from X"). Write "the owner token lives on the task in the executor" — not "ownership *now* lives on the executor instead of a plugin-local map". When a contrast genuinely aids understanding (a non-obvious choice between live alternatives), frame it against the alternative as a standing fact ("stored on the executor, NOT the tool plugin, so it survives an HMR reload"), not against the codebase's past. The same rule governs review-fix commits: the *commit message* records what the review caught; the *code comment* it touches states only the resulting truth. RFCs (`docs/rfc/`, grouped into `proposed/` / `implemented/` / `rejected/`) record the *why* behind choices a future reader would otherwise re-litigate (the vendoring policy, event-sourcing, the schema DSL are the existing examples). A PR that introduces such a decision — a new third-party runtime dependency over the vendoring default, a cross-package contract, a security/isolation model, a deviation from a documented architecture rule — writes the RFC in `implemented/` **in the same PR**, and links it from the relevant code. A proposal for future work not yet built goes in `proposed/`. A PR whose changes are mechanical, self-evident, or already covered by an existing RFC needs none — do not manufacture an RFC for a routine change. When unsure, the test is: would a competent maintainer six months from now ask "why was it done this way?" and be unable to answer from the code alone? If yes, write it. See [docs/rfc/README.md](docs/rfc/README.md) for the naming scheme and [docs/AGENTS.md](docs/AGENTS.md) for the cross-link convention. +**Document the CURRENT state — the "what" and "why" — never the PROCESS or HISTORY of how it got there.** A comment, JSDoc, or doc paragraph describes what the code *is* and why it is that way, as if it had always been so. Do NOT narrate the change that produced it: no "previously X, now Y", "changed from", "used to", "this replaces", "the old map", "renamed", "moved here", "as of this PR", or "(was …)". **In particular, NEVER name the change unit a reader cannot see — the PR, commit, or stack position that introduced the code — in a comment, JSDoc, OR a test name/description.** A `// (PR D's per-agent teardown)` aside, a `* Tests for the cancel primitive (PR C).` module doc, or an `it('… identity no longer matters')` title that only makes sense relative to a prior design are all the same violation: the reader of the current tree has no "PR D" or "old design" to anchor against, and the reference rots the moment the stack merges. Name the *mechanism* (`the session's AgentHandle teardown`), not the PR. Such phrasing rots the instant the next change lands, and a reader of the current code does not need the diff narrated in prose — that belongs in the commit message, the PR description, or an RFC (the durable home for "why we moved away from X"). Write "the owner token lives on the task in the executor" — not "ownership *now* lives on the executor instead of a plugin-local map". When a contrast genuinely aids understanding (a non-obvious choice between live alternatives), frame it against the alternative as a standing fact ("stored on the executor, NOT the tool plugin, so it survives an HMR reload"), not against the codebase's past. The same rule governs review-fix commits: the *commit message* records what the review caught; the *code comment* it touches states only the resulting truth. RFCs (`docs/rfc/`, grouped into `proposed/` / `implemented/` / `rejected/`) record the *why* behind choices a future reader would otherwise re-litigate (the vendoring policy, event-sourcing, the schema DSL are the existing examples). A PR that introduces such a decision — a new third-party runtime dependency over the vendoring default, a cross-package contract, a security/isolation model, a deviation from a documented architecture rule — writes the RFC in `implemented/` **in the same PR**, and links it from the relevant code. A proposal for future work not yet built goes in `proposed/`. A PR whose changes are mechanical, self-evident, or already covered by an existing RFC needs none — do not manufacture an RFC for a routine change. When unsure, the test is: would a competent maintainer six months from now ask "why was it done this way?" and be unable to answer from the code alone? If yes, write it. See [docs/rfc/README.md](docs/rfc/README.md) for the naming scheme and [docs/AGENTS.md](docs/AGENTS.md) for the cross-link convention. **Markdown is not hard-wrapped**: write one line per paragraph and let the editor soft-wrap. Hard line breaks mid-paragraph make docs harder to edit and diff — a one-word change reflows and re-diffs the whole paragraph. This applies to prose only: leave fenced code blocks, tables, and list structure intact (a wrapped list item folds to one line per bullet). Code comments / JSDoc are exempt — they stay under the linter's column limit. `pnpm run verify-md-wrap` (part of `doc-sync`) enforces this across `README.md`, `docs/**/*.md`, `packages/*/README.md`, and `AGENTS.md` / `packages/AGENTS.md`; `pnpm run verify-md-links` (also part of `doc-sync`) checks that every relative cross-link in those files resolves.