From 16860ec7a9aa19d8d73f51e9dece57b3d288a2d7 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Thu, 28 May 2026 11:01:53 -0700 Subject: [PATCH 1/5] agents|feat: Add /revise-comments to audit and edit existing comments Adds a `/revise-comments` command that applies the existing comment-discipline rules to a target file set, editing comments in place per the audit checklist and respecting the test-comment and `eslint-disable` carve-outs. With no argument, the command targets files changed on the current branch, including committed and uncommitted work; explicit file or directory arguments target legacy code. A `--dry-run` flag produces the per-file summary without applying edits. After processing, the command emits a per-file table classifying each comment as kept, deleted, or rewritten along with the rule that applied. --- .../content/skills/revise-comments/SKILL.md | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 packages/agents/content/skills/revise-comments/SKILL.md diff --git a/packages/agents/content/skills/revise-comments/SKILL.md b/packages/agents/content/skills/revise-comments/SKILL.md new file mode 100644 index 00000000..37b03b08 --- /dev/null +++ b/packages/agents/content/skills/revise-comments/SKILL.md @@ -0,0 +1,126 @@ +--- +name: revise-comments +description: Revise comments in target files per the comment-discipline rules, editing in place +user-invocable: true +--- + +# Revise comments + +Apply the comment-discipline audit to a target file set. Edits comments in place per the deletion rules and carve-outs. The default workflow is `/revise-comments` after a feature is implemented; explicit paths support cleaning up legacy code. + +## Invocation + +- `/revise-comments` — default target is files changed on the current branch, including committed and uncommitted work. +- `/revise-comments [...]` — explicit file or directory targets. Directories are processed recursively. +- `/revise-comments --dry-run [...]` — produce the summary without applying edits. Triage and dry-run are unified under this flag. + +## Process + +1. **Read the discipline doc.** Read `{platform_home_dir}/skills/_data/comment-discipline.md` before any edits. The audit below depends on its definitions; do not derive them from memory. + +2. **Resolve the target file set.** + + With no argument, the target is the union of files changed on the current branch (committed, staged, unstaged, and untracked). Obtain `default_branch` via `node {platform_home_dir}/skills/derive-session-context/derive-session-context.mjs`, then: + + ```bash + ( + git diff --name-only --diff-filter=AM "$(git merge-base "$default_branch" HEAD)..HEAD" + git diff --name-only --diff-filter=AM + git diff --cached --name-only --diff-filter=AM + git ls-files --others --exclude-standard + ) | sort -u + ``` + + With explicit arguments: for each argument, if it is a file, add it to the set; if it is a directory, add all comment-supporting files under it recursively. + +3. **Apply the audit per file.** Read each target file. For each comment, apply the audit checklist below. Decide one of three actions: kept, deleted, or rewritten. In normal mode, apply edits in place via the Edit tool. In `--dry-run` mode, record the proposed action without editing. + +4. **Honor the carve-outs.** See the next section. + +5. **Emit the summary.** After processing all targets, emit one table per file with non-trivial decisions. + +## Audit checklist + + + +## Carve-outs + +- **Test files.** Path contains `__tests__/` or filename matches `*.spec.*` / `*.test.*`. Inside a test file, keep comments that explain non-obvious setup, indirect assertions, or intentional skips (see the discipline doc's "Test comments" section). The deletion rules still apply to everything else inside the test file. +- **`eslint-disable` rationales.** Lines beginning with `eslint-disable`, `eslint-disable-next-line`, `eslint-disable-line`, or the block forms `/* eslint-disable */` and `/* eslint-enable */`. Keep the rationale, tightened to name only why the rule is suppressed at that line. Strip surrounding context, ticket references, and design discussion. + +## When to pause and ask + +The default is to act. Pause and ask the user when one of these holds: + +- A test comment could plausibly be non-obvious setup, but the test name already conveys the setup intent. +- An `eslint-disable` rationale sits at the boundary between tight and over-scoped. +- A file header describes potentially load-bearing architecture (composition order, threading model, invariants across functions), so the "tutorial-style file header" rule is not unambiguously applicable. + +## Summary format + +After processing, emit one Markdown table per file. Skip files with zero decisions. The same format appears in normal and `--dry-run` runs; in dry-run mode, append `(dry-run)` to the heading. + +``` +revise-comments summary + +src/lib/payload.ts +| Line | Action | Rule | +| ---- | --------- | ------------------------------------------ | +| 1-2 | deleted | 7: tutorial-style file header | +| 6 | rewritten | 2, 8: conversation + process commentary | +| 8 | kept | 10: "why" inline | +``` + +Line numbers anchor to the pre-edit file, so the summary can be cross-referenced against `git diff` output. Multiple rules on one line are comma-separated. + +## Safety + + +Never edit files outside the resolved target set. Resolve the set once at the start of the run and stay within it. Do not follow imports, expand to siblings, or touch files reachable from a touched file but outside the original set. + + +## Worked example + +A small file demonstrating multi-comment revision end-to-end. + +**Before** (`src/lib/payload.ts`): + +```ts +// Helpers for building the request payload sent to the orchestrator API. +// Originally added in PR #423 to consolidate the type narrowing logic. + +import type { Payload } from './types'; + +// Build the canonical payload shape. We discussed inlining but it's used at three call sites. +export function buildPayload(input: Input): Payload { + // react-select types value as a union; narrow to array. + const items = Array.isArray(input.value) ? input.value : [input.value]; + return { items }; +} +``` + +**After:** + +```ts +import type { Payload } from './types'; + +/** Build the canonical payload shape. */ +export function buildPayload(input: Input): Payload { + // react-select types value as a union; narrow to array. + const items = Array.isArray(input.value) ? input.value : [input.value]; + return { items }; +} +``` + +**Summary emitted:** + +``` +revise-comments summary + +src/lib/payload.ts +| Line | Action | Rule | +| ---- | --------- | ------------------------------------------ | +| 1-2 | deleted | 7: tutorial-style file header | +| 6 | rewritten | 2, 8: conversation + process commentary | +| 8 | kept | 10: "why" inline | +``` From e67c715c2dd29728df528beee6031e8b54b9fd4b Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Fri, 29 May 2026 01:15:57 -0700 Subject: [PATCH 2/5] agents|fix: Bound /revise-comments directory targets to tracked files When `/revise-comments` is invoked with a directory argument, the target set is now drawn from `git ls-files` rather than a recursive filesystem walk. `node_modules/`, `dist/`, generated bundles, and other `.gitignore`d trees are excluded automatically, so the skill no longer risks editing comments in third-party or build-output code. --- packages/agents/content/skills/revise-comments/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/agents/content/skills/revise-comments/SKILL.md b/packages/agents/content/skills/revise-comments/SKILL.md index 37b03b08..be40fe74 100644 --- a/packages/agents/content/skills/revise-comments/SKILL.md +++ b/packages/agents/content/skills/revise-comments/SKILL.md @@ -31,7 +31,7 @@ Apply the comment-discipline audit to a target file set. Edits comments in place ) | sort -u ``` - With explicit arguments: for each argument, if it is a file, add it to the set; if it is a directory, add all comment-supporting files under it recursively. + With explicit arguments: for each argument, if it is a file, add it to the set; if it is a directory, list its contents via `git ls-files ` and `git ls-files --others --exclude-standard `, then add the comment-supporting files from that listing. The `git ls-files` form respects `.gitignore`, so `node_modules/`, `dist/`, and other non-authored trees stay out of the set. 3. **Apply the audit per file.** Read each target file. For each comment, apply the audit checklist below. Decide one of three actions: kept, deleted, or rewritten. In normal mode, apply edits in place via the Edit tool. In `--dry-run` mode, record the proposed action without editing. From f5e643cc0db745122ce79644497fed0f908de934 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Fri, 29 May 2026 01:16:14 -0700 Subject: [PATCH 3/5] agents|fix: Split mixed-rule row in /revise-comments worked example The `/revise-comments` worked example's summary table now labels each deleted comment line with the specific rule that applied. A row that tagged both a tutorial-style file header (rule 7) and an ephemeral artifact reference (rule 3) under rule 7 alone is now split into two rows, matching how `comment-discipline.md` tags equivalent material in its own canonical example. --- packages/agents/content/skills/revise-comments/SKILL.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/agents/content/skills/revise-comments/SKILL.md b/packages/agents/content/skills/revise-comments/SKILL.md index be40fe74..e4dfe51b 100644 --- a/packages/agents/content/skills/revise-comments/SKILL.md +++ b/packages/agents/content/skills/revise-comments/SKILL.md @@ -66,7 +66,8 @@ revise-comments summary src/lib/payload.ts | Line | Action | Rule | | ---- | --------- | ------------------------------------------ | -| 1-2 | deleted | 7: tutorial-style file header | +| 1 | deleted | 7: tutorial-style file header | +| 2 | deleted | 3: ephemeral artifact reference | | 6 | rewritten | 2, 8: conversation + process commentary | | 8 | kept | 10: "why" inline | ``` @@ -120,7 +121,8 @@ revise-comments summary src/lib/payload.ts | Line | Action | Rule | | ---- | --------- | ------------------------------------------ | -| 1-2 | deleted | 7: tutorial-style file header | +| 1 | deleted | 7: tutorial-style file header | +| 2 | deleted | 3: ephemeral artifact reference | | 6 | rewritten | 2, 8: conversation + process commentary | | 8 | kept | 10: "why" inline | ``` From a7b67631a744a309ca3ad92f778a2c44e1e75c4d Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Fri, 29 May 2026 01:16:29 -0700 Subject: [PATCH 4/5] agents|fix: Include renamed files in /revise-comments default target The `/revise-comments` default target now includes renamed files alongside added and modified files. A file renamed on the branch without other content changes is no longer silently excluded from the audit. --- packages/agents/content/skills/revise-comments/SKILL.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/agents/content/skills/revise-comments/SKILL.md b/packages/agents/content/skills/revise-comments/SKILL.md index e4dfe51b..160307e0 100644 --- a/packages/agents/content/skills/revise-comments/SKILL.md +++ b/packages/agents/content/skills/revise-comments/SKILL.md @@ -24,9 +24,9 @@ Apply the comment-discipline audit to a target file set. Edits comments in place ```bash ( - git diff --name-only --diff-filter=AM "$(git merge-base "$default_branch" HEAD)..HEAD" - git diff --name-only --diff-filter=AM - git diff --cached --name-only --diff-filter=AM + git diff --name-only --diff-filter=AMR "$(git merge-base "$default_branch" HEAD)..HEAD" + git diff --name-only --diff-filter=AMR + git diff --cached --name-only --diff-filter=AMR git ls-files --others --exclude-standard ) | sort -u ``` From 104774501ed7aad5d85e970d2bc439645d5836b4 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Fri, 29 May 2026 02:12:18 -0700 Subject: [PATCH 5/5] agents|fix: Simplify /revise-comments default-target resolution The `/revise-comments` default target is now the set of files committed on the current branch relative to the default branch, resolved through a single `git diff --name-only` call. Uncommitted work is reached by passing explicit paths; `/revise-comments .` covers the full working tree, with `.gitignore`d trees still excluded by the directory-traversal rules. --- .../agents/content/skills/revise-comments/SKILL.md | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/agents/content/skills/revise-comments/SKILL.md b/packages/agents/content/skills/revise-comments/SKILL.md index 160307e0..d1f52ac4 100644 --- a/packages/agents/content/skills/revise-comments/SKILL.md +++ b/packages/agents/content/skills/revise-comments/SKILL.md @@ -10,7 +10,7 @@ Apply the comment-discipline audit to a target file set. Edits comments in place ## Invocation -- `/revise-comments` — default target is files changed on the current branch, including committed and uncommitted work. +- `/revise-comments` — default target is files committed on the current branch relative to the default branch. To audit uncommitted work, pass explicit paths (`/revise-comments .` covers the working tree). - `/revise-comments [...]` — explicit file or directory targets. Directories are processed recursively. - `/revise-comments --dry-run [...]` — produce the summary without applying edits. Triage and dry-run are unified under this flag. @@ -20,15 +20,10 @@ Apply the comment-discipline audit to a target file set. Edits comments in place 2. **Resolve the target file set.** - With no argument, the target is the union of files changed on the current branch (committed, staged, unstaged, and untracked). Obtain `default_branch` via `node {platform_home_dir}/skills/derive-session-context/derive-session-context.mjs`, then: + With no argument, the target is the set of files changed in commits on the current branch relative to the default branch. Obtain `$default_branch` via `node {platform_home_dir}/skills/derive-session-context/derive-session-context.mjs`, then: ```bash - ( - git diff --name-only --diff-filter=AMR "$(git merge-base "$default_branch" HEAD)..HEAD" - git diff --name-only --diff-filter=AMR - git diff --cached --name-only --diff-filter=AMR - git ls-files --others --exclude-standard - ) | sort -u + git diff --name-only "$default_branch...HEAD" ``` With explicit arguments: for each argument, if it is a file, add it to the set; if it is a directory, list its contents via `git ls-files ` and `git ls-files --others --exclude-standard `, then add the comment-supporting files from that listing. The `git ls-files` form respects `.gitignore`, so `node_modules/`, `dist/`, and other non-authored trees stay out of the set.