From fa270ad9ea2ebd4fe2ab4fb6d790f4b62eaefeec Mon Sep 17 00:00:00 2001 From: "Rob Moore (MakerX)" Date: Thu, 9 Jul 2026 17:28:33 +0800 Subject: [PATCH 01/11] feat: narration/density comment heuristics + edit-time PostToolUse hook Adds two diff-scoped heuristics to the comments check, on by default: session-narration detection ("let me", "as requested", restating what the next line does) and comment density (comments as a share of a changed file's added lines). Both judge only changed lines, exempting JSDoc, context:, and machine directives. Also adds `verifyx comments-hook`, a Claude Code PostToolUse hook that scans the text an Edit/Write introduced (parsed from the tool payload, no git) and returns exit-2 back-pressure so the agent revises low-value comments in-loop. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/checks/comment-common.ts | 60 ++++++++++ src/checks/comment-heuristics.test.ts | 53 +++++++++ src/checks/comment-heuristics.ts | 51 +++++++++ src/checks/comments.test.ts | 66 ++++++++++- src/checks/comments.ts | 151 +++++++++++++++----------- src/checks/registry.ts | 2 +- src/cli.ts | 2 + src/commands/registerChecks.ts | 23 +++- src/commands/registerCommentsHook.ts | 53 +++++++++ src/comments.ts | 7 ++ src/hook/analyze.test.ts | 44 ++++++++ src/hook/analyze.ts | 57 ++++++++++ src/hook/payload.test.ts | 39 +++++++ src/hook/payload.ts | 70 ++++++++++++ src/report.ts | 23 ++++ src/shared/comment-scan.ts | 8 +- src/shared/config.ts | 10 +- 17 files changed, 648 insertions(+), 71 deletions(-) create mode 100644 src/checks/comment-common.ts create mode 100644 src/checks/comment-heuristics.test.ts create mode 100644 src/checks/comment-heuristics.ts create mode 100644 src/commands/registerCommentsHook.ts create mode 100644 src/hook/analyze.test.ts create mode 100644 src/hook/analyze.ts create mode 100644 src/hook/payload.test.ts create mode 100644 src/hook/payload.ts diff --git a/src/checks/comment-common.ts b/src/checks/comment-common.ts new file mode 100644 index 0000000..423ddfd --- /dev/null +++ b/src/checks/comment-common.ts @@ -0,0 +1,60 @@ +import fs from 'node:fs' + +import { minimatch } from 'minimatch' + +const SCANNED_EXTENSIONS = ['.ts', '.tsx', '.cts', '.mts', '.js', '.jsx', '.mjs', '.cjs', '.yml', '.yaml'] + +// context: machine directives steer tooling, not humans, so a changed line carrying one is never a "comment" +// worth removing; `context:` is this project's durable-context escape hatch and is likewise exempt. +const MACHINE_DIRECTIVES = [ + 'oxlint-disable', + 'oxlint-enable', + '@ts-expect-error', + '@ts-ignore', + '@ts-nocheck', + 'eslint-disable', + 'eslint-enable', + 'prettier-ignore', + 'istanbul ignore', + 'v8 ignore', + 'c8 ignore', + '@vitest-environment', +] + +export type NewComment = { file: string; line: number; text: string } + +export function isCommentExempt(text: string): boolean { + const lower = text.toLowerCase() + if (MACHINE_DIRECTIVES.some((d) => lower.includes(d))) return true + const stripped = text.replace(/^\s*(?:\/\/+|\/\*+|\*|#)\s*/, '') + return stripped.toLowerCase().startsWith('context:') +} + +export function isScannedExtension(file: string): boolean { + return SCANNED_EXTENSIONS.some((ext) => file.endsWith(ext)) +} + +/** JSDoc (`/** … *\/`) is an always-allowed escape hatch, so the diff-scoped gates skip it like `context:`. */ +function isJsDoc(text: string): boolean { + return text.trimStart().startsWith('/**') +} + +/** Exempt from the diff-scoped gates: machine directives, `context:` blocks, and JSDoc. */ +export function isDiffExempt(text: string): boolean { + return isCommentExempt(text) || isJsDoc(text) +} + +export function shouldScan(file: string, ignoreGlobs: readonly string[]): boolean { + if (!isScannedExtension(file)) return false + if (ignoreGlobs.some((glob) => minimatch(file, glob))) return false + return fs.existsSync(file) +} + +export function toSingleLine(text: string): string { + return text.replace(/\s+/g, ' ').trim() +} + +/** Physical lines a scanned comment occupies (block comments carry embedded newlines in their text). */ +export function commentSpan(text: string): number { + return text.split('\n').length +} diff --git a/src/checks/comment-heuristics.test.ts b/src/checks/comment-heuristics.test.ts new file mode 100644 index 0000000..7b1e7db --- /dev/null +++ b/src/checks/comment-heuristics.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from 'vitest' + +import type { NewComment } from './comment-common.ts' +import { findCommentDensity, findNarrationComments } from './comment-heuristics.ts' + +function comment(text: string, line = 1): NewComment { + return { file: 'a.ts', line, text } +} + +describe('findNarrationComments', () => { + it('flags session-narration phrases', () => { + const flagged = [ + '// let me add the handler', + '// as requested, wire it up', + "// now I'll return the result", + '// this function returns the total', + '// First, validate the input', + '// added the retry because it was flaky', + ].map((t, i) => comment(t, i + 1)) + expect(findNarrationComments(flagged)).toHaveLength(flagged.length) + }) + + it('leaves genuine why-comments alone', () => { + const kept = [comment('// Stripe rejects amounts under 50c'), comment('// matches the wire format the gateway expects')] + expect(findNarrationComments(kept)).toEqual([]) + }) + + it('never flags exempt comments (context: / machine directives)', () => { + const exempt = [comment('// context: let me keep this durable note'), comment('// eslint-disable-next-line -- let me disable')] + expect(findNarrationComments(exempt)).toEqual([]) + }) +}) + +describe('findCommentDensity', () => { + const opts = { threshold: 0.3, minAddedLines: 10 } + + it('flags a file over the density threshold', () => { + const perFile = new Map([['a.ts', { added: 10, commentLines: 4 }]]) + const [v] = findCommentDensity(perFile, opts) + expect(v).toMatchObject({ file: 'a.ts', added: 10, commentLines: 4 }) + expect(v?.ratio).toBeCloseTo(0.4) + }) + + it('ignores tiny diffs below minAddedLines', () => { + const perFile = new Map([['a.ts', { added: 9, commentLines: 9 }]]) + expect(findCommentDensity(perFile, opts)).toEqual([]) + }) + + it('passes a file under the threshold', () => { + const perFile = new Map([['a.ts', { added: 20, commentLines: 5 }]]) + expect(findCommentDensity(perFile, opts)).toEqual([]) + }) +}) diff --git a/src/checks/comment-heuristics.ts b/src/checks/comment-heuristics.ts new file mode 100644 index 0000000..9d5225f --- /dev/null +++ b/src/checks/comment-heuristics.ts @@ -0,0 +1,51 @@ +import { isCommentExempt, type NewComment } from './comment-common.ts' + +// context: session narration — comments that describe the editing act or restate *what* the next line does, +// rather than durable *why*. These are the highest-signal tell that an agent is thinking out loud in comments. +const NARRATION_PATTERNS: RegExp[] = [ + /\blet me\b/i, + /\bas requested\b/i, + /\bas (?:you |we )?(?:asked|mentioned|discussed)\b/i, + /\bnow (?:i(?:'| a| wi)ll|i'm|we(?:'ll| will)|let(?:'s| us))\b/i, + /\bhere(?:'s| is| we)\b/i, + /\bfirst,|\bnext,|\bthen,|\bfinally,/i, + /\bthis (?:function|method|code|line|block|file|class|helper) (?:does|will|is|handles|returns|creates|adds|sets|gets)\b/i, + /\bthe following\b/i, + /\b(?:added|adding|updated|updating|changed|changing|removed|removing|refactored|fixed) (?:the|this|a|to|so|because|for)\b/i, + /\b(?:step \d|todo:|fixme:)/i, +] + +export type DensityViolation = { + file: string + /** Added lines in the changed file. */ + added: number + /** Added lines that fall inside a non-exempt comment. */ + commentLines: number + /** commentLines / added, 0–1. */ + ratio: number +} + +/** From comments already scoped to changed lines, return those whose text reads as session narration. */ +export function findNarrationComments(comments: readonly NewComment[]): NewComment[] { + return comments.filter((c) => !isCommentExempt(c.text) && NARRATION_PATTERNS.some((re) => re.test(c.text))) +} + +export type FileCommentCounts = { added: number; commentLines: number } + +/** + * Flag files where non-exempt comments make up too large a share of the added lines. Only files with at least + * `minAddedLines` added lines are considered, so tiny diffs never trip the gate. + */ +export function findCommentDensity( + perFile: ReadonlyMap, + opts: { threshold: number; minAddedLines: number }, +): DensityViolation[] { + const out: DensityViolation[] = [] + for (const [file, { added, commentLines }] of perFile) { + if (added < opts.minAddedLines) continue + const ratio = added === 0 ? 0 : commentLines / added + if (ratio >= opts.threshold) out.push({ file, added, commentLines, ratio }) + } + out.sort((a, b) => b.ratio - a.ratio || a.file.localeCompare(b.file)) + return out +} diff --git a/src/checks/comments.test.ts b/src/checks/comments.test.ts index b05ab6d..6d87797 100644 --- a/src/checks/comments.test.ts +++ b/src/checks/comments.test.ts @@ -4,12 +4,21 @@ import path from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { gitDiffAgainstBase } from '../shared/git.ts' import { runComments } from './comments.ts' +// context: the diff-scoped gates (narration/density/block-new) read git; mocking the diff keeps these tests +// hermetic and lets each case craft the exact changed lines it needs. +vi.mock('../shared/git.ts', () => ({ gitDiffAgainstBase: vi.fn(() => '') })) +const mockDiff = (diff: string): void => { + vi.mocked(gitDiffAgainstBase).mockReturnValue(diff) +} + let dir: string beforeEach(() => { dir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'verify-comments-'))) + mockDiff('') vi.spyOn(console, 'log').mockImplementation(() => {}) vi.spyOn(console, 'warn').mockImplementation(() => {}) vi.spyOn(console, 'error').mockImplementation(() => {}) @@ -26,7 +35,14 @@ function write(name: string, content: string): string { return file.replaceAll('\\', '/') } -describe('runComments', () => { +/** A unified diff that marks every line of `content` as freshly added in `file`. */ +function diffFor(file: string, content: string): string { + const lines = content.split('\n') + const body = lines.map((l) => `+${l}`).join('\n') + return `--- /dev/null\n+++ b/${file}\n@@ -0,0 +1,${lines.length} @@\n${body}\n` +} + +describe('runComments — comment blocks', () => { it('passes and is named `comments` when no block exceeds max-lines', () => { const file = write('ok.ts', ['// one line', 'const a = 1'].join('\n')) expect(runComments({ pattern: file, maxLines: 2 })).toEqual({ name: 'comments', ok: true }) @@ -47,3 +63,51 @@ describe('runComments', () => { expect(runComments({ pattern: file, maxLines: 1 }).ok).toBe(true) }) }) + +describe('runComments — narration (default on)', () => { + it('fails on a narration comment on a changed line', () => { + const content = ['const a = 1', '// let me add the handler', 'const b = 2'].join('\n') + const file = write('narr.ts', content) + mockDiff(diffFor(file, content)) + expect(runComments({ pattern: file, maxLines: 2 }).ok).toBe(false) + }) + + it('passes when --no-narration disables it', () => { + const content = ['const a = 1', '// let me add the handler', 'const b = 2'].join('\n') + const file = write('narr2.ts', content) + mockDiff(diffFor(file, content)) + expect(runComments({ pattern: file, maxLines: 2, narration: false, density: false }).ok).toBe(true) + }) +}) + +describe('runComments — density (default on)', () => { + it('fails a comment-dense changed file', () => { + const content = Array.from({ length: 12 }, (_, i) => (i % 2 === 0 ? `// note ${i}` : `const v${i} = ${i}`)).join('\n') + const file = write('dense.ts', content) + mockDiff(diffFor(file, content)) + expect(runComments({ pattern: file, maxLines: 2, narration: false }).ok).toBe(false) + }) + + it('honours a disabled density threshold', () => { + const content = Array.from({ length: 12 }, (_, i) => (i % 2 === 0 ? `// note ${i}` : `const v${i} = ${i}`)).join('\n') + const file = write('dense2.ts', content) + mockDiff(diffFor(file, content)) + expect(runComments({ pattern: file, maxLines: 2, narration: false, density: false }).ok).toBe(true) + }) +}) + +describe('runComments — block-new-comments (opt-in)', () => { + it('fails on any non-exempt comment on a changed line', () => { + const content = ['const a = 1', '// a plain comment', 'const b = 2'].join('\n') + const file = write('bn.ts', content) + mockDiff(diffFor(file, content)) + expect(runComments({ pattern: file, maxLines: 2, blockNewComments: true }).ok).toBe(false) + }) + + it('passes when the diff has no comments', () => { + const content = ['const a = 1', 'const b = 2'].join('\n') + const file = write('bn2.ts', content) + mockDiff(diffFor(file, content)) + expect(runComments({ pattern: file, maxLines: 2, blockNewComments: true }).ok).toBe(true) + }) +}) diff --git a/src/checks/comments.ts b/src/checks/comments.ts index 501d6b8..def9a5c 100644 --- a/src/checks/comments.ts +++ b/src/checks/comments.ts @@ -1,71 +1,45 @@ -import fs from 'node:fs' - -import { minimatch } from 'minimatch' - import { DEFAULT_IGNORE, DEFAULT_PATTERN, findSourceFiles, resolvePattern } from '../analyze.ts' import { findLongCommentBlocks } from '../comments.ts' -import { printCommentBlockReport } from '../report.ts' +import { printCommentBlockReport, printCommentDensityReport, printNarrationReport } from '../report.ts' import { color } from '../shared/color.ts' import { scanFileComments } from '../shared/comment-scan.ts' import { loadVerifyConfig } from '../shared/config.ts' import { parseDiffAddedLines } from '../shared/diff.ts' import { gitDiffAgainstBase } from '../shared/git.ts' +import { commentSpan, isDiffExempt, type NewComment, shouldScan, toSingleLine } from './comment-common.ts' +import { type FileCommentCounts, findCommentDensity, findNarrationComments } from './comment-heuristics.ts' import type { CheckResult } from './types.ts' const DEFAULT_MAX_COMMENT_BLOCK_LINES = 2 - -const SCANNED_EXTENSIONS = ['.ts', '.tsx', '.cts', '.mts', '.js', '.jsx', '.mjs', '.cjs', '.yml', '.yaml'] - -// context: machine directives steer tooling, not humans, so a changed line carrying one is never a "comment" -// worth removing; `context:` is this project's durable-context escape hatch and is likewise exempt. -const MACHINE_DIRECTIVES = [ - 'oxlint-disable', - 'oxlint-enable', - '@ts-expect-error', - '@ts-ignore', - '@ts-nocheck', - 'eslint-disable', - 'eslint-enable', - 'prettier-ignore', - 'istanbul ignore', - 'v8 ignore', - 'c8 ignore', - '@vitest-environment', -] - -function isCommentExempt(text: string): boolean { - const lower = text.toLowerCase() - if (MACHINE_DIRECTIVES.some((d) => lower.includes(d))) return true - const stripped = text.replace(/^\s*(?:\/\/+|\/\*+|\*|#)\s*/, '') - return stripped.toLowerCase().startsWith('context:') -} - -function shouldScan(file: string, ignoreGlobs: readonly string[]): boolean { - if (!SCANNED_EXTENSIONS.some((ext) => file.endsWith(ext))) return false - if (ignoreGlobs.some((glob) => minimatch(file, glob))) return false - return fs.existsSync(file) +const DEFAULT_DENSITY_THRESHOLD = 0.3 +const DEFAULT_MIN_ADDED_LINES = 10 + +type ChangedComments = { + /** Non-exempt comments whose first line sits on a changed line (block-new-comments + narration). */ + comments: NewComment[] + /** Per changed file: added-line count and how many of them fall inside a non-exempt comment (density). */ + perFile: Map } -function toSingleLine(text: string): string { - return text.replace(/\s+/g, ' ').trim() -} - -type NewComment = { file: string; line: number; text: string } - -// context: the --block-new-comments behaviour — flag any comment sitting on a changed line (vs HEAD, or the CI base). -function findCommentsOnChangedLines(ignoreGlobs: readonly string[]): NewComment[] { +// context: one pass over the diff feeds every diff-based gate (block-new-comments, narration, density) so we +// scan each changed file's comments only once. +function collectChangedLineComments(ignoreGlobs: readonly string[]): ChangedComments { const added = parseDiffAddedLines(gitDiffAgainstBase()) - const findings: NewComment[] = [] + const comments: NewComment[] = [] + const perFile = new Map() for (const [file, lines] of added) { if (!shouldScan(file, ignoreGlobs)) continue + let commentLines = 0 for (const comment of scanFileComments(file)) { - if (!lines.has(comment.line)) continue - if (isCommentExempt(comment.text)) continue - findings.push({ file, line: comment.line, text: toSingleLine(comment.text) }) + if (isDiffExempt(comment.text)) continue + const span = commentSpan(comment.text) + for (let l = comment.line; l < comment.line + span; l++) if (lines.has(l)) commentLines++ + if (lines.has(comment.line)) comments.push({ file, line: comment.line, text: toSingleLine(comment.text) }) } + perFile.set(file, { added: lines.size, commentLines }) } - findings.sort((a, b) => a.file.localeCompare(b.file) || a.line - b.line) - return findings + comments.sort((a, b) => a.file.localeCompare(b.file) || a.line - b.line) + return { comments, perFile } } function reportCommentsOnChangedLines(findings: readonly NewComment[]): void { @@ -83,13 +57,72 @@ export type CommentsOptions = { maxLines?: number pushback?: boolean warn?: boolean - /** Also fail on any comment on a line changed against HEAD (machine directives / context: exempt). */ + /** Also fail on any comment on a line changed against the diff base (machine directives / context: exempt). */ blockNewComments?: boolean + /** Flag session-narration comments on changed lines. Default true. */ + narration?: boolean + /** Fail files whose changed-line comment share reaches this ratio (0–1). `false`/`0` disables. Default 0.3. */ + density?: number | false + /** Minimum added lines before density applies. Default 10. */ + minAddedLines?: number +} + +function resolveDensity(opt: number | false | undefined, cfg: number | false | undefined): number { + const value = opt ?? cfg ?? DEFAULT_DENSITY_THRESHOLD + return value === false ? 0 : value +} + +function runChangedLineGates(opts: CommentsOptions): boolean { + const cfg = loadVerifyConfig().comments ?? {} + const ignoreGlobs = opts.ignore?.length ? opts.ignore : (cfg.ignore ?? []) + const narrationOn = !opts.blockNewComments && (opts.narration ?? cfg.narration ?? true) + const densityThreshold = opts.blockNewComments ? 0 : resolveDensity(opts.density, cfg.density) + const minAddedLines = opts.minAddedLines ?? cfg.minAddedLines ?? DEFAULT_MIN_ADDED_LINES + + if (!opts.blockNewComments && !narrationOn && densityThreshold === 0) return true + + const { comments, perFile } = collectChangedLineComments(ignoreGlobs) + let ok = true + + // context: --block-new-comments is the strictest gate (every new comment fails), so it subsumes the narration + // and density heuristics; when it's on we run it alone. + if (opts.blockNewComments) { + if (comments.length === 0) { + console.log(color.green('No comments on changed lines.')) + } else { + reportCommentsOnChangedLines(comments) + ok = false + } + return ok + } + + if (narrationOn) { + const narration = findNarrationComments(comments) + if (narration.length === 0) { + console.log(color.green('No narration comments on changed lines.')) + } else { + printNarrationReport(narration, { pushback: !!opts.pushback }) + ok = false + } + } + + if (densityThreshold > 0) { + const dense = findCommentDensity(perFile, { threshold: densityThreshold, minAddedLines }) + if (dense.length === 0) { + console.log(color.green('Comment density within threshold.')) + } else { + printCommentDensityReport(dense, { threshold: densityThreshold, pushback: !!opts.pushback }) + ok = false + } + } + + return ok } /** - * Native check: flag comment blocks longer than `maxLines` (JSDoc and `context:`-prefixed blocks exempt). - * With `blockNewComments`, additionally fail on any comment on a line changed against HEAD. + * Native check: flag comment blocks longer than `maxLines` (JSDoc and `context:`-prefixed blocks exempt), plus + * diff-scoped heuristics on changed lines — session narration and comment density (both on by default). With + * `blockNewComments`, instead fail on any comment on a changed line (the strictest gate). */ export function runComments(opts: CommentsOptions = {}): CheckResult { const maxLines = opts.maxLines ?? DEFAULT_MAX_COMMENT_BLOCK_LINES @@ -105,17 +138,7 @@ export function runComments(opts: CommentsOptions = {}): CheckResult { blocksOk = !!opts.warn } - let changedLinesOk = true - if (opts.blockNewComments) { - const ignoreGlobs = opts.ignore?.length ? opts.ignore : (loadVerifyConfig().comments?.ignore ?? []) - const findings = findCommentsOnChangedLines(ignoreGlobs) - if (findings.length === 0) { - console.log(color.green('No comments on changed lines.')) - } else { - reportCommentsOnChangedLines(findings) - changedLinesOk = false - } - } + const changedLinesOk = runChangedLineGates(opts) return { name: 'comments', ok: blocksOk && changedLinesOk } } diff --git a/src/checks/registry.ts b/src/checks/registry.ts index b1638c0..10373f5 100644 --- a/src/checks/registry.ts +++ b/src/checks/registry.ts @@ -25,7 +25,7 @@ export const CHECKS: Check[] = [ nativeCheck('complexity', 'Maintainability-index gate (cyclomatic + Halstead + SLOC)', true, () => runComplexity()), nativeCheck( 'comments', - 'Flag long comment blocks (JSDoc / context: exempt); --block-new-comments also fails comments on changed lines', + 'Flag long comment blocks + narration/density on changed lines (JSDoc / context: exempt); --block-new-comments fails all changed-line comments', true, () => runComments({ pushback: true }), 'verifyx comments --pushback', diff --git a/src/cli.ts b/src/cli.ts index 23b4a90..5a1a4c7 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -4,6 +4,7 @@ import { createRequire } from 'node:module' import { Command } from 'commander' import { registerChecks } from './commands/registerChecks.ts' +import { registerCommentsHook } from './commands/registerCommentsHook.ts' import { registerEject } from './commands/registerEject.ts' import { registerInit } from './commands/registerInit.ts' import { registerList } from './commands/registerList.ts' @@ -50,6 +51,7 @@ withRunOptions( }) registerChecks(program) +registerCommentsHook(program) registerList(program) registerInit(program) registerUpgradeDocs(program) diff --git a/src/commands/registerChecks.ts b/src/commands/registerChecks.ts index c1b1dbe..4c391ee 100644 --- a/src/commands/registerChecks.ts +++ b/src/commands/registerChecks.ts @@ -28,17 +28,31 @@ export function registerChecks(program: Command): void { program .command('comments') - .description('Flag long comment blocks (JSDoc / context: exempt); --block-new-comments also fails comments on changed lines') + .description( + 'Flag long comment blocks + narration/density on changed lines (JSDoc / context: exempt); --block-new-comments fails all changed-line comments', + ) .argument('[pattern]', 'glob, directory, or file to scan') .option('--max-lines ', 'maximum comment-block length', Number) .option('--pushback', 'add AI back-pressure framing to the failure message') .option('--warn', 'report without failing the run') .option('--block-new-comments', 'also fail on any comment on a line changed against HEAD') + .option('--no-narration', 'do not flag session-narration comments on changed lines') + .option('--comment-density ', 'changed-line comment-density ratio (0–1) that fails a file; 0 disables', Number) + .option('--min-added-lines ', 'minimum added lines before density applies', Number) .option('--ignore ', 'ignore glob (repeatable)', collect, []) .action( ( pattern: string | undefined, - opts: { maxLines?: number; pushback?: boolean; warn?: boolean; blockNewComments?: boolean; ignore: string[] }, + opts: { + maxLines?: number + pushback?: boolean + warn?: boolean + blockNewComments?: boolean + narration?: boolean + commentDensity?: number + minAddedLines?: number + ignore: string[] + }, ) => { finish( runComments({ @@ -47,6 +61,11 @@ export function registerChecks(program: Command): void { pushback: opts.pushback, warn: opts.warn, blockNewComments: opts.blockNewComments, + // context: commander defaults --no-narration's value to true; forward it only when explicitly + // disabled so verify.config.json's `narration` stays authoritative when the flag is absent. + narration: opts.narration === false ? false : undefined, + density: opts.commentDensity, + minAddedLines: opts.minAddedLines, ignore: opts.ignore, }).ok, ) diff --git a/src/commands/registerCommentsHook.ts b/src/commands/registerCommentsHook.ts new file mode 100644 index 0000000..14f7214 --- /dev/null +++ b/src/commands/registerCommentsHook.ts @@ -0,0 +1,53 @@ +import type { Command } from 'commander' +import { minimatch } from 'minimatch' + +import { analyzeAddedComments, formatHookFeedback, hasFindings, type HookOptions } from '../hook/analyze.ts' +import { parsePostToolUsePayload } from '../hook/payload.ts' +import { loadVerifyConfig } from '../shared/config.ts' + +const DEFAULT_MAX_LINES = 2 +const DEFAULT_DENSITY = 0.3 +const DEFAULT_MIN_ADDED_LINES = 10 + +async function readStdin(): Promise { + const chunks: Buffer[] = [] + for await (const chunk of process.stdin) chunks.push(chunk as Buffer) + return Buffer.concat(chunks).toString('utf-8') +} + +function resolveOptions(): { options: HookOptions; ignore: string[] } { + const cfg = loadVerifyConfig().comments ?? {} + const density = cfg.density === false ? 0 : (cfg.density ?? DEFAULT_DENSITY) + return { + options: { + maxLines: DEFAULT_MAX_LINES, + narration: cfg.narration ?? true, + density, + minAddedLines: cfg.minAddedLines ?? DEFAULT_MIN_ADDED_LINES, + }, + ignore: cfg.ignore ?? [], + } +} + +/** + * `verifyx comments-hook` — a Claude Code PostToolUse hook. Reads the tool payload on stdin, scans the text the + * Edit/Write introduced, and on low-value comments writes feedback to stderr and exits 2 so the agent revises + * in-loop. Stays silent (exit 0) when the edit is clean or the payload is not a scannable file edit. + */ +export function registerCommentsHook(program: Command): void { + program + .command('comments-hook', { hidden: true }) + .description('Claude Code PostToolUse hook: flag low-value comments an edit introduced (reads hook JSON on stdin)') + .action(async () => { + const target = parsePostToolUsePayload(await readStdin()) + if (!target) return + const { options, ignore } = resolveOptions() + if (ignore.some((glob) => minimatch(target.file, glob))) return + + const findings = analyzeAddedComments(target, options) + if (!hasFindings(findings)) return + + process.stderr.write(`${formatHookFeedback(findings)}\n`) + process.exitCode = 2 + }) +} diff --git a/src/comments.ts b/src/comments.ts index b905c0e..3d7040b 100644 --- a/src/comments.ts +++ b/src/comments.ts @@ -64,6 +64,13 @@ function scanFile(file: string, content: string, maxLines: number, out: CommentB } } +/** Flag comment blocks longer than `maxLines` in a single file's `content` (see {@link findLongCommentBlocks}). */ +export function findLongCommentBlocksInContent(file: string, content: string, maxLines: number): CommentBlockViolation[] { + const out: CommentBlockViolation[] = [] + scanFile(file, content, maxLines, out) + return out +} + /** * Flag comment blocks longer than `maxLines`. A block is a run of consecutive whole-line `//` * comments or a `/* ... *\/` block comment. JSDoc (`/**`) blocks and blocks whose first line diff --git a/src/hook/analyze.test.ts b/src/hook/analyze.test.ts new file mode 100644 index 0000000..905c373 --- /dev/null +++ b/src/hook/analyze.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest' + +import { analyzeAddedComments, formatHookFeedback, hasFindings, type HookOptions } from './analyze.ts' + +const opts: HookOptions = { maxLines: 2, narration: true, density: 0.3, minAddedLines: 10 } + +describe('analyzeAddedComments', () => { + it('flags a narration comment an edit introduced', () => { + const f = analyzeAddedComments({ file: 'a.ts', addedText: '// let me wire this up\nconst x = 1' }, opts) + expect(f.narration).toHaveLength(1) + expect(hasFindings(f)).toBe(true) + }) + + it('flags a long comment block', () => { + const f = analyzeAddedComments({ file: 'a.ts', addedText: ['// one', '// two', '// three', 'const x = 1'].join('\n') }, opts) + expect(f.blocks).toHaveLength(1) + }) + + it('flags a comment-dense edit', () => { + const lines = Array.from({ length: 12 }, (_, i) => (i % 2 === 0 ? `// note ${i}` : `const v${i} = ${i}`)) + const f = analyzeAddedComments({ file: 'a.ts', addedText: lines.join('\n') }, opts) + expect(f.density).not.toBeNull() + }) + + it('stays clean for a self-documenting edit', () => { + const f = analyzeAddedComments({ file: 'a.ts', addedText: 'export const total = items.reduce((a, b) => a + b, 0)' }, opts) + expect(hasFindings(f)).toBe(false) + }) + + it('never flags JSDoc or context: comments', () => { + const text = ['/**', ' * @param x the input', ' */', '// context: durable note about why', 'const x = 1'].join('\n') + const f = analyzeAddedComments({ file: 'a.ts', addedText: text }, opts) + expect(hasFindings(f)).toBe(false) + }) +}) + +describe('formatHookFeedback', () => { + it('names the file and includes the pushback', () => { + const f = analyzeAddedComments({ file: 'a.ts', addedText: '// let me do it\nconst x = 1' }, opts) + const msg = formatHookFeedback(f) + expect(msg).toContain('a.ts') + expect(msg).toContain('context:') + }) +}) diff --git a/src/hook/analyze.ts b/src/hook/analyze.ts new file mode 100644 index 0000000..485cf8a --- /dev/null +++ b/src/hook/analyze.ts @@ -0,0 +1,57 @@ +import { commentSpan, isDiffExempt, type NewComment, toSingleLine } from '../checks/comment-common.ts' +import { type DensityViolation, findCommentDensity, findNarrationComments } from '../checks/comment-heuristics.ts' +import { type CommentBlockViolation, findLongCommentBlocksInContent } from '../comments.ts' +import { scanComments } from '../shared/comment-scan.ts' +import type { HookTarget } from './payload.ts' + +export type HookOptions = { maxLines: number; narration: boolean; density: number; minAddedLines: number } + +export type HookFindings = { + file: string + blocks: CommentBlockViolation[] + narration: NewComment[] + density: DensityViolation | null +} + +export function hasFindings(f: HookFindings): boolean { + return f.blocks.length > 0 || f.narration.length > 0 || f.density !== null +} + +/** + * Apply the comment gates to the text a single edit introduced. Every comment in `addedText` is treated as new, + * so there is no diff — the fragment itself is the change. JSDoc and `context:` comments are exempt throughout. + */ +export function analyzeAddedComments(target: HookTarget, opts: HookOptions): HookFindings { + const scanned = scanComments(target.file, target.addedText) + const fresh: NewComment[] = scanned + .filter((c) => !isDiffExempt(c.text)) + .map((c) => ({ file: target.file, line: c.line, text: toSingleLine(c.text) })) + + const blocks = opts.maxLines > 0 ? findLongCommentBlocksInContent(target.file, target.addedText, opts.maxLines) : [] + const narration = opts.narration ? findNarrationComments(fresh) : [] + + let density: DensityViolation | null = null + if (opts.density > 0) { + const added = target.addedText.split('\n').length + const commentLines = scanned.filter((c) => !isDiffExempt(c.text)).reduce((n, c) => n + commentSpan(c.text), 0) + const perFile = new Map([[target.file, { added, commentLines: Math.min(commentLines, added) }]]) + density = findCommentDensity(perFile, { threshold: opts.density, minAddedLines: opts.minAddedLines })[0] ?? null + } + + return { file: target.file, blocks, narration, density } +} + +const HOOK_PUSHBACK = + 'Fix this now, in this edit, before moving on. The ONLY sanctioned way to keep a flagged comment is to prefix its first line with `context:`, which pages a human to approve it — do not do that unless the comment is genuinely durable context the code cannot express. Otherwise delete it and let the code document itself. Do NOT weaken or remove the check.' + +/** Build the stderr feedback an edit-time hook returns to the agent when it wrote low-value comments. */ +export function formatHookFeedback(f: HookFindings): string { + const parts: string[] = [`Low-value comments in your edit to ${f.file}:`] + for (const b of f.blocks) + parts.push(` • ${b.lines}-line comment block near line ${b.line} — too long; comments should explain *why*, not narrate *what*.`) + for (const n of f.narration) parts.push(` • narration comment: "${n.text}"`) + if (f.density) + parts.push(` • ${Math.round(f.density.ratio * 100)}% of the edit is comments (${f.density.commentLines}/${f.density.added} lines).`) + parts.push('', HOOK_PUSHBACK) + return parts.join('\n') +} diff --git a/src/hook/payload.test.ts b/src/hook/payload.test.ts new file mode 100644 index 0000000..6dd5446 --- /dev/null +++ b/src/hook/payload.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from 'vitest' + +import { parsePostToolUsePayload } from './payload.ts' + +function json(value: unknown): string { + return JSON.stringify(value) +} + +describe('parsePostToolUsePayload', () => { + it('parses a Write payload (whole content is added)', () => { + const raw = json({ tool_name: 'Write', tool_input: { file_path: 'a.ts', content: '// hi\nconst x = 1' } }) + expect(parsePostToolUsePayload(raw)).toEqual({ file: 'a.ts', addedText: '// hi\nconst x = 1' }) + }) + + it('parses an Edit payload from new_string', () => { + const raw = json({ tool_name: 'Edit', tool_input: { file_path: 'a.ts', old_string: 'a', new_string: '// added\nconst y = 2' } }) + expect(parsePostToolUsePayload(raw)).toEqual({ file: 'a.ts', addedText: '// added\nconst y = 2' }) + }) + + it('concatenates every MultiEdit new_string', () => { + const raw = json({ + tool_name: 'MultiEdit', + tool_input: { file_path: 'a.ts', edits: [{ new_string: '// one' }, { new_string: '// two' }] }, + }) + expect(parsePostToolUsePayload(raw)?.addedText).toBe('// one\n// two') + }) + + it('accepts the `path` key as well as `file_path`', () => { + const raw = json({ tool_name: 'Write', tool_input: { path: 'a.ts', content: 'const x = 1' } }) + expect(parsePostToolUsePayload(raw)?.file).toBe('a.ts') + }) + + it('returns null for non-file tools, unscanned extensions, empty text, and bad JSON', () => { + expect(parsePostToolUsePayload(json({ tool_name: 'Bash', tool_input: { command: 'ls' } }))).toBeNull() + expect(parsePostToolUsePayload(json({ tool_name: 'Write', tool_input: { file_path: 'a.py', content: 'x = 1' } }))).toBeNull() + expect(parsePostToolUsePayload(json({ tool_name: 'Write', tool_input: { file_path: 'a.ts', content: ' ' } }))).toBeNull() + expect(parsePostToolUsePayload('not json')).toBeNull() + }) +}) diff --git a/src/hook/payload.ts b/src/hook/payload.ts new file mode 100644 index 0000000..30377d4 --- /dev/null +++ b/src/hook/payload.ts @@ -0,0 +1,70 @@ +import { isScannedExtension } from '../checks/comment-common.ts' + +export type HookTarget = { file: string; addedText: string } + +type Json = Record + +function asRecord(value: unknown): Json | undefined { + return value && typeof value === 'object' && !Array.isArray(value) ? (value as Json) : undefined +} + +function str(value: unknown): string | undefined { + return typeof value === 'string' ? value : undefined +} + +// context: the file path key differs across Claude Code versions/tools (file_path vs path), so we accept either. +function filePathOf(input: Json): string | undefined { + return str(input.file_path) ?? str(input.path) +} + +/** The text an edit operation introduces — new_string / new_str (Edit) across naming variants. */ +function newTextOf(op: Json): string { + return str(op.new_string) ?? str(op.new_str) ?? '' +} + +function addedTextFrom(toolName: string, input: Json): string { + if (toolName === 'Write') return str(input.content) ?? '' + if (toolName === 'Edit') { + // Some shapes nest the replacement under operations[]; most carry new_string directly on the input. + const ops = Array.isArray(input.operations) ? input.operations : [] + const nested = ops + .map(asRecord) + .filter((o): o is Json => !!o) + .map(newTextOf) + return [newTextOf(input), ...nested].filter(Boolean).join('\n') + } + if (toolName === 'MultiEdit') { + const edits = Array.isArray(input.edits) ? input.edits : [] + return edits + .map(asRecord) + .filter((e): e is Json => !!e) + .map(newTextOf) + .filter(Boolean) + .join('\n') + } + return '' +} + +/** + * Extract the edited file and the text a PostToolUse Write/Edit/MultiEdit introduced. Returns null when the + * payload is not a file-editing tool, has no path, targets an unscanned extension, or introduced no text. + */ +export function parsePostToolUsePayload(raw: string): HookTarget | null { + let parsed: unknown + try { + parsed = JSON.parse(raw) + } catch { + return null + } + const payload = asRecord(parsed) + if (!payload) return null + const toolName = str(payload.tool_name) ?? '' + if (!['Write', 'Edit', 'MultiEdit'].includes(toolName)) return null + const input = asRecord(payload.tool_input) + if (!input) return null + const file = filePathOf(input) + if (!file || !isScannedExtension(file)) return null + const addedText = addedTextFrom(toolName, input) + if (addedText.trim() === '') return null + return { file, addedText } +} diff --git a/src/report.ts b/src/report.ts index bb48499..1dc60d8 100644 --- a/src/report.ts +++ b/src/report.ts @@ -1,6 +1,8 @@ import fs from 'node:fs' import type { FileScore } from './analyze.ts' +import type { NewComment } from './checks/comment-common.ts' +import type { DensityViolation } from './checks/comment-heuristics.ts' import type { CommentBlockViolation } from './comments.ts' import { forEachFunction } from './functions.ts' import { calculateCyclomaticComplexity, calculateHalstead, calculateMaintainabilityIndex, countSloc } from './metrics.ts' @@ -54,6 +56,27 @@ export function printCommentBlockReport( } } +/** Report session-narration comments found on changed lines. `pushback` adds AI back-pressure framing. */ +export function printNarrationReport(findings: readonly NewComment[], opts: { pushback: boolean }): void { + const list = findings.map((c) => ` ${c.file}:${c.line} → ${c.text}`).join('\n') + console.error( + color.red( + `\nFail: ${findings.length} narration comment(s) on changed lines.\n${list}\n\n${color.bold('These read as session narration — thinking out loud or restating *what* the next line does.')} They add no durable value and drift as the code changes. Delete them; let the code speak. If a line is genuinely durable context the code cannot express, prefix it with \`context:\`.${opts.pushback ? PUSHBACK : ''}`, + ), + ) +} + +/** Report files whose changed-line comment share exceeds the density threshold. */ +export function printCommentDensityReport(violations: readonly DensityViolation[], opts: { threshold: number; pushback: boolean }): void { + const pct = (n: number): string => `${Math.round(n * 100)}%` + const list = violations.map((v) => ` ${v.file} → ${pct(v.ratio)} (${v.commentLines}/${v.added} added lines)`).join('\n') + console.error( + color.red( + `\nFail: ${violations.length} file(s) over the ${pct(opts.threshold)} comment-density threshold on changed lines.\n${list}\n\n${color.bold('Too much of this change is comments.')} Dense comment runs usually narrate *what* the code does. Prefer self-documenting code: better names, smaller functions. Keep only comments that explain *why*; prefix genuinely durable context with \`context:\`. JSDoc (\`/** … */\`) is always allowed.${opts.pushback ? PUSHBACK : ''}`, + ), + ) +} + function printSloc(file: string, content: string): void { console.log(color.heading('SLOC')) const lines = countSloc(content) diff --git a/src/shared/comment-scan.ts b/src/shared/comment-scan.ts index 2ffe9ed..b9d917f 100644 --- a/src/shared/comment-scan.ts +++ b/src/shared/comment-scan.ts @@ -58,8 +58,12 @@ function scanYamlComments(content: string): ScannedComment[] { return out } +/** Extract every comment (with its 1-based line) from source text, dispatching on the file's extension. */ +export function scanComments(file: string, content: string): ScannedComment[] { + return isYamlFile(file) ? scanYamlComments(content) : scanCodeComments(file, content) +} + /** Extract every comment (with its 1-based line) from a source file, dispatching on extension. */ export function scanFileComments(file: string): ScannedComment[] { - const content = fs.readFileSync(file, 'utf-8') - return isYamlFile(file) ? scanYamlComments(content) : scanCodeComments(file, content) + return scanComments(file, fs.readFileSync(file, 'utf-8')) } diff --git a/src/shared/config.ts b/src/shared/config.ts index b277c81..51670f2 100644 --- a/src/shared/config.ts +++ b/src/shared/config.ts @@ -9,7 +9,15 @@ export type ForbiddenStringsRule = { } export type VerifyConfig = { - comments?: { ignore?: string[] } + comments?: { + ignore?: string[] + /** Flag session-narration comments on changed lines. Default true. */ + narration?: boolean + /** Changed-line comment-density ratio (0–1) that fails a file; `false`/`0` disables. Default 0.3. */ + density?: number | false + /** Minimum added lines before density applies. Default 10. */ + minAddedLines?: number + } hardcodedColors?: { ignore?: string[]; root?: string } forbiddenStrings?: ForbiddenStringsRule[] } From 1832cdbd5956f9558fcdf24e21b6b831478d0bc9 Mon Sep 17 00:00:00 2001 From: "Rob Moore (MakerX)" Date: Thu, 9 Jul 2026 17:28:43 +0800 Subject: [PATCH 02/11] feat: scaffold comment hook + prune-comments skill via verifyx init verifyx init now merges the comments-hook PostToolUse entry into .claude/settings.json (idempotent, never clobbering existing keys; --no-comment-hook to skip) and drops a prune-comments remediation skill into .claude/skills and .agent-skills. Dogfoods both into this repo. Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/settings.json | 15 ++++++ .claude/skills/prune-comments/SKILL.md | 57 ++++++++++++++++++++++ src/commands/registerInit.ts | 19 ++++++-- src/scaffold/agentFiles.ts | 3 ++ src/scaffold/claudeSettings.test.ts | 61 ++++++++++++++++++++++++ src/scaffold/claudeSettings.ts | 60 +++++++++++++++++++++++ src/scaffold/init.ts | 6 +++ templates/skills/prune-comments/SKILL.md | 57 ++++++++++++++++++++++ 8 files changed, 274 insertions(+), 4 deletions(-) create mode 100644 .claude/settings.json create mode 100644 .claude/skills/prune-comments/SKILL.md create mode 100644 src/scaffold/claudeSettings.test.ts create mode 100644 src/scaffold/claudeSettings.ts create mode 100644 templates/skills/prune-comments/SKILL.md diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..3db1b90 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,15 @@ +{ + "hooks": { + "PostToolUse": [ + { + "matcher": "Edit|Write|MultiEdit", + "hooks": [ + { + "type": "command", + "command": "npx verifyx comments-hook" + } + ] + } + ] + } +} diff --git a/.claude/skills/prune-comments/SKILL.md b/.claude/skills/prune-comments/SKILL.md new file mode 100644 index 0000000..b268a92 --- /dev/null +++ b/.claude/skills/prune-comments/SKILL.md @@ -0,0 +1,57 @@ +--- +name: prune-comments +description: Remove low-value code comments across the current change. Use when asked to "prune comments", "clean up comments", or after the verify comments check flags narration, dense comments, or long comment blocks. +--- + +Capable coding agents love to narrate their work in comments. This skill removes that noise +from the current change while protecting the comments that genuinely earn their place. + +## 1. Find the flagged comments + +Run the comments check over the branch diff (the binary is `verifyx`, since `verify` is a +Windows builtin): + +``` +npx verifyx comments --block-new-comments --pushback +``` + +That lists every comment on a changed line. For a softer pass that only flags narration and +dense comment runs (the defaults), drop `--block-new-comments`. + +## 2. Delete or keep — the criteria + +**Delete** a comment when any of these hold: + +- It narrates _what_ the next line does (`// increment the counter`, `// return the result`). +- It narrates the editing session (`// let me add a handler`, `// as requested`, `// now we…`). +- It restates a name already in the code (`// user service` above `class UserService`). +- It is a commented-out block of old code. +- Removing it loses nothing a competent reader couldn't recover from the code itself. + +**Keep** a comment when it explains _why_, not _what_: + +- A non-obvious constraint, workaround, or business rule (`// Stripe rejects amounts under 50c`). +- A deliberate deviation a reader would otherwise "fix" and break. +- A link to an issue/spec that explains a surprising choice. + +If a kept comment is genuinely durable context the code cannot express, prefix its first line +with `context:` so the check leaves it alone: + +```ts +// context: retries must stay at 3 — the upstream gateway drops the connection after the 4th. +``` + +Machine directives (`eslint-disable`, `@ts-expect-error`, …) and JSDoc (`/** … */`) are +always allowed and never need pruning. + +## 3. Prefer fixing the code over keeping the comment + +When a comment exists because the code is unclear, the better fix is usually to rename a +variable, extract a well-named function, or simplify — then delete the comment. Reach for +`context:` only when the code truly cannot carry the meaning. + +## 4. Re-run until clean + +After editing, run the check again and repeat until it passes. Do not silence the check, +widen its ignore globs, or mark low-value comments `context:` to slip them past the gate — +`context:` pages a human to approve the comment. diff --git a/src/commands/registerInit.ts b/src/commands/registerInit.ts index 32aa9a7..67f50f3 100644 --- a/src/commands/registerInit.ts +++ b/src/commands/registerInit.ts @@ -34,9 +34,10 @@ type InitCliOptions = { select: string[] claude?: boolean agents?: boolean + commentHook?: boolean } -type Selections = { checks: string[]; targets: AgentTarget[]; defaultsOnly: boolean } +type Selections = { checks: string[]; targets: AgentTarget[]; defaultsOnly: boolean; commentHook: boolean } async function resolveSelections(opts: InitCliOptions): Promise { const nonInteractive = !!opts.yes || !process.stdin.isTTY @@ -48,6 +49,7 @@ async function resolveSelections(opts: InitCliOptions): Promise { checks: opts.select.length > 0 ? opts.select : recommendedChecks().map((c) => c.name), targets, defaultsOnly: !!opts.defaultsOnly, + commentHook: opts.commentHook !== false, } } @@ -70,7 +72,15 @@ async function resolveSelections(opts: InitCliOptions): Promise { { name: 'claude', message: 'Claude (.claude/skills + CLAUDE.md)', enabled: true }, { name: 'agents', message: 'Other agents (.agent-skills + AGENTS.md)', enabled: false }, ]) - return { checks, targets, defaultsOnly } + + // The edit-time hook is Claude-specific; only offer it when Claude is a target. + const commentHook = targets.includes('claude') + ? (await ask('select', 'Enable the edit-time comment hook (Claude PostToolUse)?', [ + { name: 'yes', message: 'Yes — flag low-value comments the moment a file is edited', enabled: true }, + { name: 'no', message: 'No — rely on the verify/CI comments check only' }, + ])) === 'yes' + : false + return { checks, targets, defaultsOnly, commentHook } } function report(result: InitResult, defaultsOnly: boolean): void { @@ -114,11 +124,12 @@ export function registerInit(program: Command): void { .option('--select ', 'preselect a check by name (repeatable, non-interactive)', collect, []) .option('--no-claude', 'do not write .claude/ files (non-interactive)') .option('--agents', 'also write .agent-skills/ files (non-interactive)') + .option('--no-comment-hook', 'do not register the edit-time comment PostToolUse hook') .action(async (opts: InitCliOptions) => { const cwd = process.cwd() - const { checks, targets, defaultsOnly } = await resolveSelections(opts) + const { checks, targets, defaultsOnly, commentHook } = await resolveSelections(opts) - const result = applyInit({ cwd, checks, targets, defaultsOnly }) + const result = applyInit({ cwd, checks, targets, defaultsOnly, commentHook }) report(result, defaultsOnly) if (result.devDeps.length > 0) { diff --git a/src/scaffold/agentFiles.ts b/src/scaffold/agentFiles.ts index 15ab9e3..16463a9 100644 --- a/src/scaffold/agentFiles.ts +++ b/src/scaffold/agentFiles.ts @@ -29,14 +29,17 @@ function readTemplate(relativePath: string): string { export function writeAgentFiles(cwd: string, targets: readonly AgentTarget[]): ManagedFileResult[] { const results: ManagedFileResult[] = [] const skill = readTemplate('skills/verify/SKILL.md') + const pruneSkill = readTemplate('skills/prune-comments/SKILL.md') const guidance = readTemplate('verify-guidance.md') if (targets.includes('claude')) { writeManaged(path.join(cwd, '.claude', 'skills', 'verify', 'SKILL.md'), skill, results) + writeManaged(path.join(cwd, '.claude', 'skills', 'prune-comments', 'SKILL.md'), pruneSkill, results) ensurePointer(path.join(cwd, 'CLAUDE.md'), guidance, POINTER_MARKER, results) } if (targets.includes('agents')) { writeManaged(path.join(cwd, '.agent-skills', 'verify', 'SKILL.md'), skill, results) + writeManaged(path.join(cwd, '.agent-skills', 'prune-comments', 'SKILL.md'), pruneSkill, results) ensurePointer(path.join(cwd, 'AGENTS.md'), guidance, POINTER_MARKER, results) } diff --git a/src/scaffold/claudeSettings.test.ts b/src/scaffold/claudeSettings.test.ts new file mode 100644 index 0000000..d037d50 --- /dev/null +++ b/src/scaffold/claudeSettings.test.ts @@ -0,0 +1,61 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' + +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { ensureClaudeHook, HOOK_COMMAND } from './claudeSettings.ts' +import type { ManagedFileResult } from './writeManaged.ts' + +let dir: string +const settingsPath = (): string => path.join(dir, '.claude', 'settings.json') +const read = (): Record => JSON.parse(fs.readFileSync(settingsPath(), 'utf-8')) + +beforeEach(() => { + dir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'verify-settings-'))) +}) +afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }) +}) + +function run(): ManagedFileResult[] { + const results: ManagedFileResult[] = [] + ensureClaudeHook(dir, results) + return results +} + +describe('ensureClaudeHook', () => { + it('creates settings.json with the PostToolUse hook when none exists', () => { + const [result] = run() + expect(result).toEqual({ path: settingsPath(), action: 'created' }) + const command = read().hooks as { PostToolUse: { hooks: { command: string }[] }[] } + expect(command.PostToolUse[0]?.hooks[0]?.command).toBe(HOOK_COMMAND) + }) + + it('merges the hook without clobbering existing settings', () => { + fs.mkdirSync(path.join(dir, '.claude'), { recursive: true }) + fs.writeFileSync(settingsPath(), JSON.stringify({ model: 'opus', hooks: { PreToolUse: [] } }, null, 2)) + const [result] = run() + expect(result?.action).toBe('updated') + const settings = read() as { model: string; hooks: { PreToolUse: unknown[]; PostToolUse: unknown[] } } + expect(settings.model).toBe('opus') + expect(settings.hooks.PreToolUse).toEqual([]) + expect(settings.hooks.PostToolUse).toHaveLength(1) + }) + + it('is idempotent — re-running does not duplicate the hook', () => { + run() + const [result] = run() + expect(result?.action).toBe('unchanged') + const settings = read() as { hooks: { PostToolUse: unknown[] } } + expect(settings.hooks.PostToolUse).toHaveLength(1) + }) + + it('leaves unparseable settings untouched', () => { + fs.mkdirSync(path.join(dir, '.claude'), { recursive: true }) + fs.writeFileSync(settingsPath(), '{ not valid json') + const [result] = run() + expect(result?.action).toBe('unchanged') + expect(fs.readFileSync(settingsPath(), 'utf-8')).toBe('{ not valid json') + }) +}) diff --git a/src/scaffold/claudeSettings.ts b/src/scaffold/claudeSettings.ts new file mode 100644 index 0000000..e08b692 --- /dev/null +++ b/src/scaffold/claudeSettings.ts @@ -0,0 +1,60 @@ +import fs from 'node:fs' +import path from 'node:path' + +import type { ManagedFileResult } from './writeManaged.ts' + +// context: the edit-time comment gate is a PostToolUse hook that runs after the agent edits a file; the marker +// lets us detect our own entry so re-running init never duplicates it. +export const HOOK_COMMAND = 'npx verifyx comments-hook' +const HOOK_MATCHER = 'Edit|Write|MultiEdit' + +type HookEntry = { type?: string; command?: string } +type HookGroup = { matcher?: string; hooks?: HookEntry[] } +type ClaudeSettings = { hooks?: { PostToolUse?: HookGroup[] } & Record } & Record + +function hasOurHook(settings: ClaudeSettings): boolean { + const groups = settings.hooks?.PostToolUse ?? [] + return groups.some((g) => (g.hooks ?? []).some((h) => (h.command ?? '').includes(HOOK_COMMAND))) +} + +function addOurHook(settings: ClaudeSettings): void { + const hooks = (settings.hooks ??= {}) + const post = (hooks.PostToolUse ??= []) + post.push({ matcher: HOOK_MATCHER, hooks: [{ type: 'command', command: HOOK_COMMAND }] }) +} + +/** + * Ensure the project's `.claude/settings.json` registers the `verifyx comments-hook` PostToolUse hook, so the + * comment gate fires the moment an agent edits a file. Merges into existing settings — never rewrites unrelated + * keys — and is idempotent (detects our entry by its command). Leaves unparseable settings untouched. + */ +export function ensureClaudeHook(cwd: string, results: ManagedFileResult[]): void { + const file = path.join(cwd, '.claude', 'settings.json') + + if (!fs.existsSync(file)) { + const settings: ClaudeSettings = {} + addOurHook(settings) + fs.mkdirSync(path.dirname(file), { recursive: true }) + fs.writeFileSync(file, `${JSON.stringify(settings, null, 2)}\n`) + results.push({ path: file, action: 'created' }) + return + } + + let settings: ClaudeSettings + try { + settings = JSON.parse(fs.readFileSync(file, 'utf-8')) as ClaudeSettings + } catch { + // context: never clobber settings we can't parse — leave the file for the user to fix and report no change. + results.push({ path: file, action: 'unchanged' }) + return + } + + if (hasOurHook(settings)) { + results.push({ path: file, action: 'unchanged' }) + return + } + + addOurHook(settings) + fs.writeFileSync(file, `${JSON.stringify(settings, null, 2)}\n`) + results.push({ path: file, action: 'updated' }) +} diff --git a/src/scaffold/init.ts b/src/scaffold/init.ts index 8b2c496..9245dc9 100644 --- a/src/scaffold/init.ts +++ b/src/scaffold/init.ts @@ -3,6 +3,7 @@ import path from 'node:path' import { getCheck } from '../checks/registry.ts' import type { Check } from '../checks/types.ts' import { type AgentTarget, writeAgentFiles } from './agentFiles.ts' +import { ensureClaudeHook } from './claudeSettings.ts' import { ensureKnipIgnores } from './knipConfig.ts' import { addVerifyScripts } from './packageScripts.ts' import type { ManagedFileResult } from './writeManaged.ts' @@ -14,6 +15,8 @@ export type InitOptions = { targets: readonly AgentTarget[] /** When true, do not write `verify:*` scripts — rely on `verify`'s built-in defaults. */ defaultsOnly: boolean + /** Register the edit-time comment gate as a Claude PostToolUse hook (claude target only). Default off. */ + commentHook?: boolean } export type InitResult = { @@ -40,6 +43,9 @@ export function applyInit(opts: InitOptions): InitResult { const agentFiles = writeAgentFiles(opts.cwd, opts.targets) + // The PostToolUse hook is Claude-specific; other agents have no universal edit-time equivalent. + if (opts.commentHook && opts.targets.includes('claude')) ensureClaudeHook(opts.cwd, agentFiles) + // context: with unused-code selected, teach knip to ignore the other external tools verifyx runs at runtime. if (opts.checks.includes('unused-code')) { const toolDeps = opts.checks diff --git a/templates/skills/prune-comments/SKILL.md b/templates/skills/prune-comments/SKILL.md new file mode 100644 index 0000000..b268a92 --- /dev/null +++ b/templates/skills/prune-comments/SKILL.md @@ -0,0 +1,57 @@ +--- +name: prune-comments +description: Remove low-value code comments across the current change. Use when asked to "prune comments", "clean up comments", or after the verify comments check flags narration, dense comments, or long comment blocks. +--- + +Capable coding agents love to narrate their work in comments. This skill removes that noise +from the current change while protecting the comments that genuinely earn their place. + +## 1. Find the flagged comments + +Run the comments check over the branch diff (the binary is `verifyx`, since `verify` is a +Windows builtin): + +``` +npx verifyx comments --block-new-comments --pushback +``` + +That lists every comment on a changed line. For a softer pass that only flags narration and +dense comment runs (the defaults), drop `--block-new-comments`. + +## 2. Delete or keep — the criteria + +**Delete** a comment when any of these hold: + +- It narrates _what_ the next line does (`// increment the counter`, `// return the result`). +- It narrates the editing session (`// let me add a handler`, `// as requested`, `// now we…`). +- It restates a name already in the code (`// user service` above `class UserService`). +- It is a commented-out block of old code. +- Removing it loses nothing a competent reader couldn't recover from the code itself. + +**Keep** a comment when it explains _why_, not _what_: + +- A non-obvious constraint, workaround, or business rule (`// Stripe rejects amounts under 50c`). +- A deliberate deviation a reader would otherwise "fix" and break. +- A link to an issue/spec that explains a surprising choice. + +If a kept comment is genuinely durable context the code cannot express, prefix its first line +with `context:` so the check leaves it alone: + +```ts +// context: retries must stay at 3 — the upstream gateway drops the connection after the 4th. +``` + +Machine directives (`eslint-disable`, `@ts-expect-error`, …) and JSDoc (`/** … */`) are +always allowed and never need pruning. + +## 3. Prefer fixing the code over keeping the comment + +When a comment exists because the code is unclear, the better fix is usually to rename a +variable, extract a well-named function, or simplify — then delete the comment. Reach for +`context:` only when the code truly cannot carry the meaning. + +## 4. Re-run until clean + +After editing, run the check again and repeat until it passes. Do not silence the check, +widen its ignore globs, or mark low-value comments `context:` to slip them past the gate — +`context:` pages a human to approve the comment. From c2255c2e54daf88f5e8fe4d06f2a075f8fd3cb96 Mon Sep 17 00:00:00 2001 From: "Rob Moore (MakerX)" Date: Thu, 9 Jul 2026 17:28:43 +0800 Subject: [PATCH 03/11] docs: document comment heuristics, edit-time hook, and prune skill Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 59 +++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index befd121..cf6b458 100644 --- a/README.md +++ b/README.md @@ -114,18 +114,18 @@ Flags on the bare `verifyx` command: ## Built-in checks -| Check | Kind | What it catches | -| ------------------- | -------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `complexity` | native | Maintainability-index gate (cyclomatic complexity + Halstead volume + SLOC). Fails files below a threshold. | -| `comments` | native | Flags comment blocks taller than `--max-lines` (default 2), to push for self-documenting code. JSDoc and `context:`-prefixed blocks are always allowed. Add `--block-new-comments` to also fail any comment on a changed line (vs `HEAD` locally, the PR base in CI). | -| `hardcoded-colors` | native | Literal hex / `0x` colour values in source (cross-platform; suggests using design tokens). | -| `forbidden-strings` | native | Disallowed JSON config values, from rules in your verify config. | -| `lint` | external | Linting; auto-fixes locally, checks in CI ([oxlint](https://oxc.rs)). | -| `format` | external | Formatting; writes locally, checks in CI ([oxfmt](https://oxc.rs)). | -| `check-types` | external | TypeScript type check (`tsc --noEmit`); skips when there is no `tsconfig.json`. | -| `unused-code` | external | Unused files, exports and dependencies ([knip](https://knip.dev)). | -| `circular-deps` | external | Circular dependencies ([skott](https://github.com/antoine-coulon/skott)). | -| `duplicate-code` | external | Copy-paste detection ([jscpd](https://github.com/kucherenko/jscpd)). | +| Check | Kind | What it catches | +| ------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `complexity` | native | Maintainability-index gate (cyclomatic complexity + Halstead volume + SLOC). Fails files below a threshold. | +| `comments` | native | Flags comment blocks taller than `--max-lines` (default 2), plus **session narration** and high **comment density** on changed lines (both on by default), to push for self-documenting code. JSDoc and `context:`-prefixed blocks are always allowed. Add `--block-new-comments` to fail _every_ comment on a changed line (vs `HEAD` locally, the PR base in CI). | +| `hardcoded-colors` | native | Literal hex / `0x` colour values in source (cross-platform; suggests using design tokens). | +| `forbidden-strings` | native | Disallowed JSON config values, from rules in your verify config. | +| `lint` | external | Linting; auto-fixes locally, checks in CI ([oxlint](https://oxc.rs)). | +| `format` | external | Formatting; writes locally, checks in CI ([oxfmt](https://oxc.rs)). | +| `check-types` | external | TypeScript type check (`tsc --noEmit`); skips when there is no `tsconfig.json`. | +| `unused-code` | external | Unused files, exports and dependencies ([knip](https://knip.dev)). | +| `circular-deps` | external | Circular dependencies ([skott](https://github.com/antoine-coulon/skott)). | +| `duplicate-code` | external | Copy-paste detection ([jscpd](https://github.com/kucherenko/jscpd)). | External checks shell out to their tool and **skip gracefully when it is not installed**; `verifyx init` installs the ones you opt into. They run the tool from your local `node_modules/.bin` regardless of how `verifyx` was invoked. `oxlint`/`oxfmt`/`tsc` are resolved if present; the rest are declared as optional `peerDependencies`. @@ -174,8 +174,18 @@ Capable coding models (Opus 4.8 very much included) love to narrate their work: - `--max-lines `: fail on comment blocks longer than `n` lines (default 2). - `--pushback`: reframe the failure message as back-pressure aimed at an AI agent (see below). - `--warn`: report the long-block violations without failing the run. -- `--block-new-comments`: also fail on any comment on a changed line (vs `HEAD` locally, the PR base in CI; see below). -- `--ignore `: exclude files (repeatable; the `--block-new-comments` gate also reads `comments.ignore` from your [verify config](#configuration)). +- `--no-narration`: turn off the session-narration gate (on by default; see below). +- `--comment-density `: the changed-line comment share (0–1) that fails a file (default `0.3`; `0` disables). +- `--min-added-lines `: skip the density gate on diffs smaller than `n` added lines (default 10). +- `--block-new-comments`: fail on _any_ comment on a changed line — the strictest gate; supersedes narration/density (vs `HEAD` locally, the PR base in CI; see below). +- `--ignore `: exclude files (repeatable; the diff-scoped gates also read `comments.ignore` from your [verify config](#configuration)). + +Beyond the long-block check, two **diff-scoped heuristics** run on changed lines by default (so they judge only what a change introduces, never legacy comments): + +- **Narration** flags comments that think out loud or restate _what_ the next line does — `// let me add the handler`, `// as requested`, `// this function returns the total`. These are the clearest tell that an agent is narrating its edit rather than documenting a decision. +- **Density** fails a changed file when comments make up too large a share of its added lines (default 30%, on diffs of 10+ added lines). Dense comment runs almost always narrate rather than explain. + +Both honour the same escape hatches as everything else (JSDoc, `context:`, machine directives). Tune or disable them per repo via [verify config](#configuration). **`--pushback` is the clever bit.** An AI agent that hits a failing check will often take the path of least resistance and just delete or weaken the check to make the run pass. So rather than a dry error, the pushback message tells the agent that the _only_ sanctioned way to keep the comment is to prefix it with `context:`, and that doing so **pages a human to approve it**. Confronted with a real person's time on the line, the agent tends to reconsider and remove the low-value comment instead of gaming the gate. It is a small piece of prompt-engineering baked into a lint failure, and in practice it stops agents silencing the check far more reliably than a plain error does. @@ -190,6 +200,25 @@ Two escape hatches keep genuinely useful comments alive. **JSDoc** (`/** … */` const timeoutMs = timeout * 1000 ``` +#### Edit-time hook (Claude PostToolUse) + +The `comments` check runs at verify/CI time — the end of a task. To close the loop tighter, `verifyx init` can also register an **edit-time gate** as a Claude Code [PostToolUse hook](https://code.claude.com/docs/en/hooks), so the same rules fire the moment an agent writes a file: + +```jsonc +// .claude/settings.json (added by `verifyx init`; opt out with --no-comment-hook) +{ + "hooks": { + "PostToolUse": [{ "matcher": "Edit|Write|MultiEdit", "hooks": [{ "type": "command", "command": "npx verifyx comments-hook" }] }], + }, +} +``` + +`verifyx comments-hook` reads the tool payload on stdin, scans only the text that edit introduced (no git needed), and on a long block, narration, or dense comments writes back-pressure to stderr and exits `2` — which Claude surfaces to the agent in the same turn, so it revises before the code ever reaches you. The hook is Claude-specific (other agents have no universal edit-time equivalent); those rely on the verify/CI check and the `prune-comments` skill. + +#### `prune-comments` skill + +`verifyx init` also drops a **`prune-comments`** skill (`.claude/skills` + `.agent-skills`) — a remediation companion to the gate. Ask an agent to "prune comments" and it runs the check over the branch diff, then walks the delete/keep criteria (with `context:` as the sanctioned escape hatch) to clean up low-value comments across an existing change. + ### `hardcoded-colors` ```sh @@ -271,7 +300,7 @@ The **native** checks that take persistent settings read them from a `verify.con ```jsonc { "verify": { - "comments": { "ignore": ["**/*.generated.ts"] }, + "comments": { "ignore": ["**/*.generated.ts"], "narration": true, "density": 0.3, "minAddedLines": 10 }, "hardcodedColors": { "root": "src", "ignore": ["**/tokens.ts"] }, "forbiddenStrings": [{ "file": "app.json", "paths": ["env.LOG_LEVEL"], "disallowed": "debug" }], }, From 4b02bede0913363bb9e6970641cc17ef77f570fb Mon Sep 17 00:00:00 2001 From: "Rob Moore (MakerX)" Date: Thu, 9 Jul 2026 19:10:30 +0800 Subject: [PATCH 04/11] feat: comment scope (diff|all) + block-all axes, and claude-comment-gate parity Splits the comments check into two orthogonal axes: --scope diff (default, changed lines) vs all (whole codebase), and --block-all (fail every comment in scope) vs the default heuristics. --block-new-comments becomes an alias for --scope diff --block-all. Long-block detection is now diff-scoped by default like narration/density. Adopts the missing claude-comment-gate signals: H2 LLM punctuation tells (em-dash/curly quotes), H3 net-increase guard (a comment trim is never flagged) and blank-line exclusion from the density denominator. The edit-time hook now reuses the canonical PUSHBACK from report.ts instead of a weaker paraphrase. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/checks/comment-collect.ts | 76 +++++++++++ src/checks/comment-common.ts | 8 ++ src/checks/comment-heuristics.test.ts | 24 +++- src/checks/comment-heuristics.ts | 27 +++- src/checks/comments.test.ts | 15 ++- src/checks/comments.ts | 185 ++++++++++++-------------- src/checks/registry.ts | 2 +- src/commands/registerChecks.ts | 18 ++- src/commands/registerCommentsHook.ts | 1 + src/hook/analyze.test.ts | 9 +- src/hook/analyze.ts | 31 +++-- src/report.ts | 34 +++-- src/shared/comment-scan.ts | 6 - src/shared/config.ts | 10 +- src/shared/diff.ts | 23 ++++ 15 files changed, 310 insertions(+), 159 deletions(-) create mode 100644 src/checks/comment-collect.ts diff --git a/src/checks/comment-collect.ts b/src/checks/comment-collect.ts new file mode 100644 index 0000000..e4cf8ed --- /dev/null +++ b/src/checks/comment-collect.ts @@ -0,0 +1,76 @@ +import fs from 'node:fs' + +import { type CommentBlockViolation, findLongCommentBlocksInContent } from '../comments.ts' +import { scanComments } from '../shared/comment-scan.ts' +import { parseDiffAddedLines, parseDiffRemovedLines } from '../shared/diff.ts' +import { gitDiffAgainstBase } from '../shared/git.ts' +import { commentSpan, isDiffExempt, looksLikeCommentLine, type NewComment, shouldScan, toSingleLine } from './comment-common.ts' +import type { FileCommentCounts } from './comment-heuristics.ts' + +export type CommentCandidates = { + /** Long comment blocks in scope. */ + blocks: CommentBlockViolation[] + /** Non-exempt comments in scope (feeds narration + --block-all). */ + comments: NewComment[] + /** Per-file line counts for the density gate. */ + perFile: Map +} + +const isBlank = (line: string | undefined): boolean => (line ?? '').trim() === '' + +const blockIntersects = (block: CommentBlockViolation, changed: ReadonlySet): boolean => { + for (let l = block.line; l < block.line + block.lines; l++) if (changed.has(l)) return true + return false +} + +const sortComments = (comments: NewComment[]): NewComment[] => comments.sort((a, b) => a.file.localeCompare(b.file) || a.line - b.line) + +// context: scope `all` judges every comment in the matched files — the whole-codebase audit / prune surface. +export function gatherAllComments(files: readonly string[], maxLines: number): CommentCandidates { + const blocks: CommentBlockViolation[] = [] + const comments: NewComment[] = [] + const perFile = new Map() + for (const file of files) { + const content = fs.readFileSync(file, 'utf-8') + blocks.push(...findLongCommentBlocksInContent(file, content, maxLines)) + let commentLines = 0 + for (const c of scanComments(file, content)) { + if (isDiffExempt(c.text)) continue + commentLines += commentSpan(c.text) + comments.push({ file, line: c.line, text: toSingleLine(c.text) }) + } + const nonBlank = content.split('\n').filter((l) => !isBlank(l)).length + perFile.set(file, { added: nonBlank, commentLines, removedComments: 0 }) + } + return { blocks, comments: sortComments(comments), perFile } +} + +// context: scope `diff` judges only comments touching changed lines (vs the diff base) — one pass feeds every gate. +export function gatherDiffComments(ignoreGlobs: readonly string[], maxLines: number): CommentCandidates { + const diff = gitDiffAgainstBase() + const added = parseDiffAddedLines(diff) + const removedByFile = parseDiffRemovedLines(diff) + const blocks: CommentBlockViolation[] = [] + const comments: NewComment[] = [] + const perFile = new Map() + for (const [file, changed] of added) { + if (!shouldScan(file, ignoreGlobs)) continue + const content = fs.readFileSync(file, 'utf-8') + const fileLines = content.split('\n') + for (const block of findLongCommentBlocksInContent(file, content, maxLines)) { + if (blockIntersects(block, changed)) blocks.push(block) + } + let commentLines = 0 + for (const c of scanComments(file, content)) { + if (isDiffExempt(c.text)) continue + const span = commentSpan(c.text) + for (let l = c.line; l < c.line + span; l++) if (changed.has(l)) commentLines++ + if (changed.has(c.line)) comments.push({ file, line: c.line, text: toSingleLine(c.text) }) + } + let addedNonBlank = 0 + for (const l of changed) if (!isBlank(fileLines[l - 1])) addedNonBlank++ + const removedComments = (removedByFile.get(file) ?? []).filter(looksLikeCommentLine).length + perFile.set(file, { added: addedNonBlank, commentLines, removedComments }) + } + return { blocks, comments: sortComments(comments), perFile } +} diff --git a/src/checks/comment-common.ts b/src/checks/comment-common.ts index 423ddfd..ef4274a 100644 --- a/src/checks/comment-common.ts +++ b/src/checks/comment-common.ts @@ -58,3 +58,11 @@ export function toSingleLine(text: string): string { export function commentSpan(text: string): number { return text.split('\n').length } + +// context: a line-level comment test for *removed* diff lines — we can't AST a line that no longer exists on +// disk, so the density net-increase guard falls back to prefix matching (mirrors the checker's is_comment_line). +export function looksLikeCommentLine(line: string): boolean { + const s = line.trimStart() + if (s.startsWith('#!')) return false + return s.startsWith('//') || s.startsWith('/*') || s.startsWith('*') || s.startsWith('#') || s.startsWith('--') +} diff --git a/src/checks/comment-heuristics.test.ts b/src/checks/comment-heuristics.test.ts index 7b1e7db..d8b4afd 100644 --- a/src/checks/comment-heuristics.test.ts +++ b/src/checks/comment-heuristics.test.ts @@ -29,25 +29,37 @@ describe('findNarrationComments', () => { const exempt = [comment('// context: let me keep this durable note'), comment('// eslint-disable-next-line -- let me disable')] expect(findNarrationComments(exempt)).toEqual([]) }) + + it('flags LLM punctuation tells (em-dash, curly quotes)', () => { + const tells = [comment('// wraps the call — then retries'), comment('// uses the “fast” path')] + expect(findNarrationComments(tells)).toHaveLength(2) + }) }) describe('findCommentDensity', () => { const opts = { threshold: 0.3, minAddedLines: 10 } + const counts = ( + added: number, + commentLines: number, + removedComments = 0, + ): Map => + new Map([['a.ts', { added, commentLines, removedComments }]]) it('flags a file over the density threshold', () => { - const perFile = new Map([['a.ts', { added: 10, commentLines: 4 }]]) - const [v] = findCommentDensity(perFile, opts) + const [v] = findCommentDensity(counts(10, 4), opts) expect(v).toMatchObject({ file: 'a.ts', added: 10, commentLines: 4 }) expect(v?.ratio).toBeCloseTo(0.4) }) it('ignores tiny diffs below minAddedLines', () => { - const perFile = new Map([['a.ts', { added: 9, commentLines: 9 }]]) - expect(findCommentDensity(perFile, opts)).toEqual([]) + expect(findCommentDensity(counts(9, 9), opts)).toEqual([]) }) it('passes a file under the threshold', () => { - const perFile = new Map([['a.ts', { added: 20, commentLines: 5 }]]) - expect(findCommentDensity(perFile, opts)).toEqual([]) + expect(findCommentDensity(counts(20, 5), opts)).toEqual([]) + }) + + it('skips a net comment trim (removed >= added comments)', () => { + expect(findCommentDensity(counts(10, 4, 4), opts)).toEqual([]) }) }) diff --git a/src/checks/comment-heuristics.ts b/src/checks/comment-heuristics.ts index 9d5225f..d35aa79 100644 --- a/src/checks/comment-heuristics.ts +++ b/src/checks/comment-heuristics.ts @@ -15,6 +15,10 @@ const NARRATION_PATTERNS: RegExp[] = [ /\b(?:step \d|todo:|fixme:)/i, ] +// context: em-dash and curly quotes are high-precision LLM tells — models emit them freely, hand-typed code +// comments rarely do. Borrowed from claude-comment-gate's H2 punctuation check. +const LLM_PUNCT_TELL = /[—‘’“”]/ + export type DensityViolation = { file: string /** Added lines in the changed file. */ @@ -25,24 +29,35 @@ export type DensityViolation = { ratio: number } -/** From comments already scoped to changed lines, return those whose text reads as session narration. */ +/** From comments in scope, return those whose text reads as session narration or carries an LLM punctuation tell. */ export function findNarrationComments(comments: readonly NewComment[]): NewComment[] { - return comments.filter((c) => !isCommentExempt(c.text) && NARRATION_PATTERNS.some((re) => re.test(c.text))) + return comments.filter( + (c) => !isCommentExempt(c.text) && (NARRATION_PATTERNS.some((re) => re.test(c.text)) || LLM_PUNCT_TELL.test(c.text)), + ) } -export type FileCommentCounts = { added: number; commentLines: number } +export type FileCommentCounts = { + /** Non-blank lines in scope (added non-blank lines for diff scope; non-blank file lines for `all`). */ + added: number + /** Lines in scope that fall inside a non-exempt comment. */ + commentLines: number + /** Comment lines the change removed — the density gate skips a net comment *trim* (diff scope only; 0 for `all`). */ + removedComments: number +} /** - * Flag files where non-exempt comments make up too large a share of the added lines. Only files with at least - * `minAddedLines` added lines are considered, so tiny diffs never trip the gate. + * Flag files where non-exempt comments make up too large a share of the lines in scope. Files with fewer than + * `minAddedLines` lines are skipped (tiny diffs never trip the gate), as is any change that removes at least as + * many comment lines as it adds — a net trim cannot be adding bloat. */ export function findCommentDensity( perFile: ReadonlyMap, opts: { threshold: number; minAddedLines: number }, ): DensityViolation[] { const out: DensityViolation[] = [] - for (const [file, { added, commentLines }] of perFile) { + for (const [file, { added, commentLines, removedComments }] of perFile) { if (added < opts.minAddedLines) continue + if (commentLines <= removedComments) continue const ratio = added === 0 ? 0 : commentLines / added if (ratio >= opts.threshold) out.push({ file, added, commentLines, ratio }) } diff --git a/src/checks/comments.test.ts b/src/checks/comments.test.ts index 6d87797..f33986d 100644 --- a/src/checks/comments.test.ts +++ b/src/checks/comments.test.ts @@ -42,25 +42,30 @@ function diffFor(file: string, content: string): string { return `--- /dev/null\n+++ b/${file}\n@@ -0,0 +1,${lines.length} @@\n${body}\n` } -describe('runComments — comment blocks', () => { +describe('runComments — comment blocks (scope: all)', () => { it('passes and is named `comments` when no block exceeds max-lines', () => { const file = write('ok.ts', ['// one line', 'const a = 1'].join('\n')) - expect(runComments({ pattern: file, maxLines: 2 })).toEqual({ name: 'comments', ok: true }) + expect(runComments({ pattern: file, maxLines: 2, scope: 'all' })).toEqual({ name: 'comments', ok: true }) }) it('fails on a comment block longer than max-lines', () => { const file = write('bad.ts', ['// one', '// two', '// three', 'const a = 1'].join('\n')) - expect(runComments({ pattern: file, maxLines: 2 })).toEqual({ name: 'comments', ok: false }) + expect(runComments({ pattern: file, maxLines: 2, scope: 'all' })).toEqual({ name: 'comments', ok: false }) }) it('reports without failing under warn', () => { const file = write('warn.ts', ['// one', '// two', '// three', 'const a = 1'].join('\n')) - expect(runComments({ pattern: file, maxLines: 2, warn: true }).ok).toBe(true) + expect(runComments({ pattern: file, maxLines: 2, warn: true, scope: 'all' }).ok).toBe(true) }) it('leaves JSDoc and context: blocks alone regardless of length', () => { const file = write('exempt.ts', ['/**', ' * a', ' * b', ' * c', ' */', '// context: durable', '// more', 'const a = 1'].join('\n')) - expect(runComments({ pattern: file, maxLines: 1 }).ok).toBe(true) + expect(runComments({ pattern: file, maxLines: 1, scope: 'all' }).ok).toBe(true) + }) + + it('does not flag a whole-file block under the default diff scope with an empty diff', () => { + const file = write('diffscope.ts', ['// one', '// two', '// three', 'const a = 1'].join('\n')) + expect(runComments({ pattern: file, maxLines: 2 }).ok).toBe(true) }) }) diff --git a/src/checks/comments.ts b/src/checks/comments.ts index def9a5c..86bf9f3 100644 --- a/src/checks/comments.ts +++ b/src/checks/comments.ts @@ -1,69 +1,38 @@ import { DEFAULT_IGNORE, DEFAULT_PATTERN, findSourceFiles, resolvePattern } from '../analyze.ts' -import { findLongCommentBlocks } from '../comments.ts' -import { printCommentBlockReport, printCommentDensityReport, printNarrationReport } from '../report.ts' +import { + type CommentScope, + printBlockAllReport, + printCommentBlockReport, + printCommentDensityReport, + printNarrationReport, +} from '../report.ts' import { color } from '../shared/color.ts' -import { scanFileComments } from '../shared/comment-scan.ts' import { loadVerifyConfig } from '../shared/config.ts' -import { parseDiffAddedLines } from '../shared/diff.ts' -import { gitDiffAgainstBase } from '../shared/git.ts' -import { commentSpan, isDiffExempt, type NewComment, shouldScan, toSingleLine } from './comment-common.ts' -import { type FileCommentCounts, findCommentDensity, findNarrationComments } from './comment-heuristics.ts' +import { type CommentCandidates, gatherAllComments, gatherDiffComments } from './comment-collect.ts' +import { findCommentDensity, findNarrationComments } from './comment-heuristics.ts' import type { CheckResult } from './types.ts' const DEFAULT_MAX_COMMENT_BLOCK_LINES = 2 const DEFAULT_DENSITY_THRESHOLD = 0.3 const DEFAULT_MIN_ADDED_LINES = 10 -type ChangedComments = { - /** Non-exempt comments whose first line sits on a changed line (block-new-comments + narration). */ - comments: NewComment[] - /** Per changed file: added-line count and how many of them fall inside a non-exempt comment (density). */ - perFile: Map -} - -// context: one pass over the diff feeds every diff-based gate (block-new-comments, narration, density) so we -// scan each changed file's comments only once. -function collectChangedLineComments(ignoreGlobs: readonly string[]): ChangedComments { - const added = parseDiffAddedLines(gitDiffAgainstBase()) - const comments: NewComment[] = [] - const perFile = new Map() - for (const [file, lines] of added) { - if (!shouldScan(file, ignoreGlobs)) continue - let commentLines = 0 - for (const comment of scanFileComments(file)) { - if (isDiffExempt(comment.text)) continue - const span = commentSpan(comment.text) - for (let l = comment.line; l < comment.line + span; l++) if (lines.has(l)) commentLines++ - if (lines.has(comment.line)) comments.push({ file, line: comment.line, text: toSingleLine(comment.text) }) - } - perFile.set(file, { added: lines.size, commentLines }) - } - comments.sort((a, b) => a.file.localeCompare(b.file) || a.line - b.line) - return { comments, perFile } -} - -function reportCommentsOnChangedLines(findings: readonly NewComment[]): void { - console.error(color.red('\nComments on changed lines:\n')) - for (const { file, line, text } of findings) console.error(` ${file}:${line} → ${text}`) - console.error(color.red(`\nTotal: ${findings.length} comment(s)`)) - console.error( - '\nWith --block-new-comments, every comment on a changed line fails this gate, whether you added or edited it. Remove them and let the code document itself.', - ) -} - export type CommentsOptions = { pattern?: string ignore?: readonly string[] maxLines?: number pushback?: boolean warn?: boolean - /** Also fail on any comment on a line changed against the diff base (machine directives / context: exempt). */ + /** Which comments the gates judge. Default `diff` (changed lines); `all` scans whole files. */ + scope?: CommentScope + /** Fail every non-exempt comment in scope, not just heuristic hits. */ + blockAll?: boolean + /** Back-compat alias for `scope: 'diff'` + `blockAll: true`. */ blockNewComments?: boolean - /** Flag session-narration comments on changed lines. Default true. */ + /** Flag session-narration comments. Default true. */ narration?: boolean - /** Fail files whose changed-line comment share reaches this ratio (0–1). `false`/`0` disables. Default 0.3. */ + /** Comment-density ratio (0–1) that fails a file; `false`/`0` disables. Default 0.3. */ density?: number | false - /** Minimum added lines before density applies. Default 10. */ + /** Minimum lines in scope before density applies. Default 10. */ minAddedLines?: number } @@ -72,73 +41,83 @@ function resolveDensity(opt: number | false | undefined, cfg: number | false | u return value === false ? 0 : value } -function runChangedLineGates(opts: CommentsOptions): boolean { +const whereText = (scope: CommentScope): string => (scope === 'all' ? 'in the codebase' : 'on changed lines') + +function gather(opts: CommentsOptions, scope: CommentScope, ignoreGlobs: readonly string[], maxLines: number): CommentCandidates { + if (scope === 'all') { + const files = findSourceFiles(resolvePattern(opts.pattern ?? DEFAULT_PATTERN), [...DEFAULT_IGNORE, ...(opts.ignore ?? [])]) + return gatherAllComments(files, maxLines) + } + return gatherDiffComments(ignoreGlobs, maxLines) +} + +/** + * Native check for low-value comments across two orthogonal axes: **scope** (`diff` — only comments on changed + * lines, the default; or `all` — every comment in the matched files) and **strictness** (heuristics by default — + * long blocks, session narration, comment density; or `blockAll`, which fails every comment in scope). JSDoc, + * `context:` blocks, and machine directives are always exempt. + */ +export function runComments(opts: CommentsOptions = {}): CheckResult { const cfg = loadVerifyConfig().comments ?? {} + const maxLines = opts.maxLines ?? DEFAULT_MAX_COMMENT_BLOCK_LINES const ignoreGlobs = opts.ignore?.length ? opts.ignore : (cfg.ignore ?? []) - const narrationOn = !opts.blockNewComments && (opts.narration ?? cfg.narration ?? true) - const densityThreshold = opts.blockNewComments ? 0 : resolveDensity(opts.density, cfg.density) + const scope: CommentScope = opts.scope ?? (opts.blockNewComments ? 'diff' : undefined) ?? cfg.scope ?? 'diff' + const blockAll = opts.blockAll ?? opts.blockNewComments ?? cfg.blockAll ?? false + const narrationOn = !blockAll && (opts.narration ?? cfg.narration ?? true) + const densityThreshold = blockAll ? 0 : resolveDensity(opts.density, cfg.density) const minAddedLines = opts.minAddedLines ?? cfg.minAddedLines ?? DEFAULT_MIN_ADDED_LINES + const pushback = !!opts.pushback - if (!opts.blockNewComments && !narrationOn && densityThreshold === 0) return true - - const { comments, perFile } = collectChangedLineComments(ignoreGlobs) - let ok = true + const candidates = gather(opts, scope, ignoreGlobs, maxLines) - // context: --block-new-comments is the strictest gate (every new comment fails), so it subsumes the narration - // and density heuristics; when it's on we run it alone. - if (opts.blockNewComments) { - if (comments.length === 0) { - console.log(color.green('No comments on changed lines.')) - } else { - reportCommentsOnChangedLines(comments) - ok = false + // context: --block-all is the strictest gate (every comment fails), so it stands alone — no heuristics run. + if (blockAll) { + if (candidates.comments.length === 0) { + console.log(color.green(`No comments ${whereText(scope)}.`)) + return { name: 'comments', ok: true } } - return ok + printBlockAllReport(candidates.comments, { scope, pushback }) + return { name: 'comments', ok: false } } - if (narrationOn) { - const narration = findNarrationComments(comments) - if (narration.length === 0) { - console.log(color.green('No narration comments on changed lines.')) - } else { - printNarrationReport(narration, { pushback: !!opts.pushback }) - ok = false - } - } - - if (densityThreshold > 0) { - const dense = findCommentDensity(perFile, { threshold: densityThreshold, minAddedLines }) - if (dense.length === 0) { - console.log(color.green('Comment density within threshold.')) - } else { - printCommentDensityReport(dense, { threshold: densityThreshold, pushback: !!opts.pushback }) - ok = false - } - } - - return ok + let ok = true + ok = reportBlocks(candidates, maxLines, { pushback, warn: !!opts.warn }) && ok + if (narrationOn) ok = reportNarration(candidates, scope, pushback) && ok + if (densityThreshold > 0) ok = reportDensity(candidates, scope, pushback, densityThreshold, minAddedLines) && ok + return { name: 'comments', ok } } -/** - * Native check: flag comment blocks longer than `maxLines` (JSDoc and `context:`-prefixed blocks exempt), plus - * diff-scoped heuristics on changed lines — session narration and comment density (both on by default). With - * `blockNewComments`, instead fail on any comment on a changed line (the strictest gate). - */ -export function runComments(opts: CommentsOptions = {}): CheckResult { - const maxLines = opts.maxLines ?? DEFAULT_MAX_COMMENT_BLOCK_LINES - const pattern = opts.pattern ?? DEFAULT_PATTERN - const files = findSourceFiles(resolvePattern(pattern), [...DEFAULT_IGNORE, ...(opts.ignore ?? [])]) - - const blocks = findLongCommentBlocks(files, maxLines) - let blocksOk = true - if (blocks.length === 0) { +function reportBlocks(candidates: CommentCandidates, maxLines: number, opts: { pushback: boolean; warn: boolean }): boolean { + if (candidates.blocks.length === 0) { console.log(color.green(`No comment block over ${maxLines} line(s)`)) - } else { - printCommentBlockReport(blocks, maxLines, { pushback: !!opts.pushback, warn: !!opts.warn }) - blocksOk = !!opts.warn + return true } + printCommentBlockReport(candidates.blocks, maxLines, opts) + return opts.warn +} - const changedLinesOk = runChangedLineGates(opts) +function reportNarration(candidates: CommentCandidates, scope: CommentScope, pushback: boolean): boolean { + const narration = findNarrationComments(candidates.comments) + if (narration.length === 0) { + console.log(color.green(`No narration comments ${whereText(scope)}.`)) + return true + } + printNarrationReport(narration, { scope, pushback }) + return false +} - return { name: 'comments', ok: blocksOk && changedLinesOk } +function reportDensity( + candidates: CommentCandidates, + scope: CommentScope, + pushback: boolean, + threshold: number, + minAddedLines: number, +): boolean { + const dense = findCommentDensity(candidates.perFile, { threshold, minAddedLines }) + if (dense.length === 0) { + console.log(color.green('Comment density within threshold.')) + return true + } + printCommentDensityReport(dense, { threshold, scope, pushback }) + return false } diff --git a/src/checks/registry.ts b/src/checks/registry.ts index 10373f5..bd6efa1 100644 --- a/src/checks/registry.ts +++ b/src/checks/registry.ts @@ -25,7 +25,7 @@ export const CHECKS: Check[] = [ nativeCheck('complexity', 'Maintainability-index gate (cyclomatic + Halstead + SLOC)', true, () => runComplexity()), nativeCheck( 'comments', - 'Flag long comment blocks + narration/density on changed lines (JSDoc / context: exempt); --block-new-comments fails all changed-line comments', + 'Flag low-value comments — long blocks + narration/density (JSDoc / context: exempt); --scope diff|all, --block-all for a zero-comment gate', true, () => runComments({ pushback: true }), 'verifyx comments --pushback', diff --git a/src/commands/registerChecks.ts b/src/commands/registerChecks.ts index 4c391ee..3cd76e5 100644 --- a/src/commands/registerChecks.ts +++ b/src/commands/registerChecks.ts @@ -29,16 +29,18 @@ export function registerChecks(program: Command): void { program .command('comments') .description( - 'Flag long comment blocks + narration/density on changed lines (JSDoc / context: exempt); --block-new-comments fails all changed-line comments', + 'Flag low-value comments: long blocks + narration/density (JSDoc / context: exempt). --scope diff|all sets whether it judges changed lines or the whole codebase; --block-all fails every comment in scope', ) - .argument('[pattern]', 'glob, directory, or file to scan') + .argument('[pattern]', 'glob, directory, or file to scan (used by --scope all)') .option('--max-lines ', 'maximum comment-block length', Number) .option('--pushback', 'add AI back-pressure framing to the failure message') .option('--warn', 'report without failing the run') - .option('--block-new-comments', 'also fail on any comment on a line changed against HEAD') - .option('--no-narration', 'do not flag session-narration comments on changed lines') - .option('--comment-density ', 'changed-line comment-density ratio (0–1) that fails a file; 0 disables', Number) - .option('--min-added-lines ', 'minimum added lines before density applies', Number) + .option('--scope ', 'which comments to judge: diff (changed lines) or all (whole codebase)') + .option('--block-all', 'fail every non-exempt comment in scope, not just heuristic hits') + .option('--block-new-comments', 'alias for --scope diff --block-all') + .option('--no-narration', 'do not flag session-narration comments') + .option('--comment-density ', 'comment-density ratio (0–1) that fails a file; 0 disables', Number) + .option('--min-added-lines ', 'minimum lines in scope before density applies', Number) .option('--ignore ', 'ignore glob (repeatable)', collect, []) .action( ( @@ -47,6 +49,8 @@ export function registerChecks(program: Command): void { maxLines?: number pushback?: boolean warn?: boolean + scope?: string + blockAll?: boolean blockNewComments?: boolean narration?: boolean commentDensity?: number @@ -60,6 +64,8 @@ export function registerChecks(program: Command): void { maxLines: opts.maxLines, pushback: opts.pushback, warn: opts.warn, + scope: opts.scope === 'all' ? 'all' : opts.scope === 'diff' ? 'diff' : undefined, + blockAll: opts.blockAll, blockNewComments: opts.blockNewComments, // context: commander defaults --no-narration's value to true; forward it only when explicitly // disabled so verify.config.json's `narration` stays authoritative when the flag is absent. diff --git a/src/commands/registerCommentsHook.ts b/src/commands/registerCommentsHook.ts index 14f7214..1e23990 100644 --- a/src/commands/registerCommentsHook.ts +++ b/src/commands/registerCommentsHook.ts @@ -24,6 +24,7 @@ function resolveOptions(): { options: HookOptions; ignore: string[] } { narration: cfg.narration ?? true, density, minAddedLines: cfg.minAddedLines ?? DEFAULT_MIN_ADDED_LINES, + blockAll: cfg.blockAll ?? false, }, ignore: cfg.ignore ?? [], } diff --git a/src/hook/analyze.test.ts b/src/hook/analyze.test.ts index 905c373..e258308 100644 --- a/src/hook/analyze.test.ts +++ b/src/hook/analyze.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest' import { analyzeAddedComments, formatHookFeedback, hasFindings, type HookOptions } from './analyze.ts' -const opts: HookOptions = { maxLines: 2, narration: true, density: 0.3, minAddedLines: 10 } +const opts: HookOptions = { maxLines: 2, narration: true, density: 0.3, minAddedLines: 10, blockAll: false } describe('analyzeAddedComments', () => { it('flags a narration comment an edit introduced', () => { @@ -32,6 +32,13 @@ describe('analyzeAddedComments', () => { const f = analyzeAddedComments({ file: 'a.ts', addedText: text }, opts) expect(hasFindings(f)).toBe(false) }) + + it('under blockAll, flags every non-exempt comment (and skips heuristics)', () => { + const f = analyzeAddedComments({ file: 'a.ts', addedText: '// a plain useful why\nconst x = 1' }, { ...opts, blockAll: true }) + expect(f.all).toHaveLength(1) + expect(f.blocks).toEqual([]) + expect(hasFindings(f)).toBe(true) + }) }) describe('formatHookFeedback', () => { diff --git a/src/hook/analyze.ts b/src/hook/analyze.ts index 485cf8a..84a285e 100644 --- a/src/hook/analyze.ts +++ b/src/hook/analyze.ts @@ -1,57 +1,60 @@ import { commentSpan, isDiffExempt, type NewComment, toSingleLine } from '../checks/comment-common.ts' import { type DensityViolation, findCommentDensity, findNarrationComments } from '../checks/comment-heuristics.ts' import { type CommentBlockViolation, findLongCommentBlocksInContent } from '../comments.ts' +import { PUSHBACK } from '../report.ts' import { scanComments } from '../shared/comment-scan.ts' import type { HookTarget } from './payload.ts' -export type HookOptions = { maxLines: number; narration: boolean; density: number; minAddedLines: number } +export type HookOptions = { maxLines: number; narration: boolean; density: number; minAddedLines: number; blockAll: boolean } export type HookFindings = { file: string + /** Every non-exempt comment the edit added — populated only under blockAll. */ + all: NewComment[] blocks: CommentBlockViolation[] narration: NewComment[] density: DensityViolation | null } export function hasFindings(f: HookFindings): boolean { - return f.blocks.length > 0 || f.narration.length > 0 || f.density !== null + return f.all.length > 0 || f.blocks.length > 0 || f.narration.length > 0 || f.density !== null } /** * Apply the comment gates to the text a single edit introduced. Every comment in `addedText` is treated as new, - * so there is no diff — the fragment itself is the change. JSDoc and `context:` comments are exempt throughout. + * so there is no diff — the fragment itself is the change (always diff-scoped). JSDoc and `context:` are exempt. */ export function analyzeAddedComments(target: HookTarget, opts: HookOptions): HookFindings { - const scanned = scanComments(target.file, target.addedText) - const fresh: NewComment[] = scanned + const fresh: NewComment[] = scanComments(target.file, target.addedText) .filter((c) => !isDiffExempt(c.text)) .map((c) => ({ file: target.file, line: c.line, text: toSingleLine(c.text) })) + if (opts.blockAll) return { file: target.file, all: fresh, blocks: [], narration: [], density: null } + const blocks = opts.maxLines > 0 ? findLongCommentBlocksInContent(target.file, target.addedText, opts.maxLines) : [] const narration = opts.narration ? findNarrationComments(fresh) : [] let density: DensityViolation | null = null if (opts.density > 0) { - const added = target.addedText.split('\n').length - const commentLines = scanned.filter((c) => !isDiffExempt(c.text)).reduce((n, c) => n + commentSpan(c.text), 0) - const perFile = new Map([[target.file, { added, commentLines: Math.min(commentLines, added) }]]) + const added = target.addedText.split('\n').filter((l) => l.trim() !== '').length + const commentLines = scanComments(target.file, target.addedText) + .filter((c) => !isDiffExempt(c.text)) + .reduce((n, c) => n + commentSpan(c.text), 0) + const perFile = new Map([[target.file, { added, commentLines: Math.min(commentLines, added), removedComments: 0 }]]) density = findCommentDensity(perFile, { threshold: opts.density, minAddedLines: opts.minAddedLines })[0] ?? null } - return { file: target.file, blocks, narration, density } + return { file: target.file, all: [], blocks, narration, density } } -const HOOK_PUSHBACK = - 'Fix this now, in this edit, before moving on. The ONLY sanctioned way to keep a flagged comment is to prefix its first line with `context:`, which pages a human to approve it — do not do that unless the comment is genuinely durable context the code cannot express. Otherwise delete it and let the code document itself. Do NOT weaken or remove the check.' - /** Build the stderr feedback an edit-time hook returns to the agent when it wrote low-value comments. */ export function formatHookFeedback(f: HookFindings): string { const parts: string[] = [`Low-value comments in your edit to ${f.file}:`] + for (const c of f.all) parts.push(` • comment (--block-all): "${c.text}"`) for (const b of f.blocks) parts.push(` • ${b.lines}-line comment block near line ${b.line} — too long; comments should explain *why*, not narrate *what*.`) for (const n of f.narration) parts.push(` • narration comment: "${n.text}"`) if (f.density) parts.push(` • ${Math.round(f.density.ratio * 100)}% of the edit is comments (${f.density.commentLines}/${f.density.added} lines).`) - parts.push('', HOOK_PUSHBACK) - return parts.join('\n') + return parts.join('\n') + PUSHBACK } diff --git a/src/report.ts b/src/report.ts index 1dc60d8..15a443c 100644 --- a/src/report.ts +++ b/src/report.ts @@ -38,9 +38,13 @@ export function printFailure(failing: readonly FileScore[], threshold: number): ) } -const PUSHBACK = +export const PUSHBACK = '\n\n⚠️ THIS IS YOUR LAST CHANCE TO RECONSIDER BEFORE INVOLVING A HUMAN. Adding `context:` pages a real person to approve this comment. DO NOT WASTE THEIR TIME. You had BETTER BE RIGHT that this comment is genuinely necessary — that it explains *why*, not *what*, and that the code cannot simply be made self-explanatory. If in doubt, delete the comment.' +/** Where the gate looked, for report wording. */ +export type CommentScope = 'diff' | 'all' +const where = (scope: CommentScope): string => (scope === 'all' ? 'in the codebase' : 'on changed lines') + /** Report comment blocks longer than the configured maximum. `warn` prints without failing; `pushback` adds AI back-pressure framing. */ export function printCommentBlockReport( violations: readonly CommentBlockViolation[], @@ -56,23 +60,37 @@ export function printCommentBlockReport( } } -/** Report session-narration comments found on changed lines. `pushback` adds AI back-pressure framing. */ -export function printNarrationReport(findings: readonly NewComment[], opts: { pushback: boolean }): void { +/** Report every non-exempt comment in scope (the `--block-all` gate). `pushback` adds AI back-pressure framing. */ +export function printBlockAllReport(findings: readonly NewComment[], opts: { scope: CommentScope; pushback: boolean }): void { const list = findings.map((c) => ` ${c.file}:${c.line} → ${c.text}`).join('\n') console.error( color.red( - `\nFail: ${findings.length} narration comment(s) on changed lines.\n${list}\n\n${color.bold('These read as session narration — thinking out loud or restating *what* the next line does.')} They add no durable value and drift as the code changes. Delete them; let the code speak. If a line is genuinely durable context the code cannot express, prefix it with \`context:\`.${opts.pushback ? PUSHBACK : ''}`, + `\nFail: ${findings.length} comment(s) ${where(opts.scope)}.\n${list}\n\nWith --block-all, every comment ${where(opts.scope)} fails this gate. Remove them and let the code document itself. Machine directives and \`context:\` comments are exempt.${opts.pushback ? PUSHBACK : ''}`, ), ) } -/** Report files whose changed-line comment share exceeds the density threshold. */ -export function printCommentDensityReport(violations: readonly DensityViolation[], opts: { threshold: number; pushback: boolean }): void { +/** Report session-narration comments found in scope. `pushback` adds AI back-pressure framing. */ +export function printNarrationReport(findings: readonly NewComment[], opts: { scope: CommentScope; pushback: boolean }): void { + const list = findings.map((c) => ` ${c.file}:${c.line} → ${c.text}`).join('\n') + console.error( + color.red( + `\nFail: ${findings.length} narration comment(s) ${where(opts.scope)}.\n${list}\n\n${color.bold('These read as session narration — thinking out loud or restating *what* the next line does.')} They add no durable value and drift as the code changes. Delete them; let the code speak. If a line is genuinely durable context the code cannot express, prefix it with \`context:\`.${opts.pushback ? PUSHBACK : ''}`, + ), + ) +} + +/** Report files whose comment share in scope exceeds the density threshold. */ +export function printCommentDensityReport( + violations: readonly DensityViolation[], + opts: { threshold: number; scope: CommentScope; pushback: boolean }, +): void { const pct = (n: number): string => `${Math.round(n * 100)}%` - const list = violations.map((v) => ` ${v.file} → ${pct(v.ratio)} (${v.commentLines}/${v.added} added lines)`).join('\n') + const unit = opts.scope === 'all' ? 'lines' : 'added lines' + const list = violations.map((v) => ` ${v.file} → ${pct(v.ratio)} (${v.commentLines}/${v.added} ${unit})`).join('\n') console.error( color.red( - `\nFail: ${violations.length} file(s) over the ${pct(opts.threshold)} comment-density threshold on changed lines.\n${list}\n\n${color.bold('Too much of this change is comments.')} Dense comment runs usually narrate *what* the code does. Prefer self-documenting code: better names, smaller functions. Keep only comments that explain *why*; prefix genuinely durable context with \`context:\`. JSDoc (\`/** … */\`) is always allowed.${opts.pushback ? PUSHBACK : ''}`, + `\nFail: ${violations.length} file(s) over the ${pct(opts.threshold)} comment-density threshold ${where(opts.scope)}.\n${list}\n\n${color.bold('Too much of this is comments.')} Dense comment runs usually narrate *what* the code does. Prefer self-documenting code: better names, smaller functions. Keep only comments that explain *why*; prefix genuinely durable context with \`context:\`. JSDoc (\`/** … */\`) is always allowed.${opts.pushback ? PUSHBACK : ''}`, ), ) } diff --git a/src/shared/comment-scan.ts b/src/shared/comment-scan.ts index b9d917f..ddc319a 100644 --- a/src/shared/comment-scan.ts +++ b/src/shared/comment-scan.ts @@ -1,4 +1,3 @@ -import fs from 'node:fs' import path from 'node:path' import ts from 'typescript' @@ -62,8 +61,3 @@ function scanYamlComments(content: string): ScannedComment[] { export function scanComments(file: string, content: string): ScannedComment[] { return isYamlFile(file) ? scanYamlComments(content) : scanCodeComments(file, content) } - -/** Extract every comment (with its 1-based line) from a source file, dispatching on extension. */ -export function scanFileComments(file: string): ScannedComment[] { - return scanComments(file, fs.readFileSync(file, 'utf-8')) -} diff --git a/src/shared/config.ts b/src/shared/config.ts index 51670f2..2941661 100644 --- a/src/shared/config.ts +++ b/src/shared/config.ts @@ -11,11 +11,15 @@ export type ForbiddenStringsRule = { export type VerifyConfig = { comments?: { ignore?: string[] - /** Flag session-narration comments on changed lines. Default true. */ + /** Which comments the gates judge: `diff` (changed lines only) or `all` (whole codebase). Default `diff`. */ + scope?: 'diff' | 'all' + /** Fail every comment in scope, not just heuristic hits. Default false. */ + blockAll?: boolean + /** Flag session-narration comments. Default true. */ narration?: boolean - /** Changed-line comment-density ratio (0–1) that fails a file; `false`/`0` disables. Default 0.3. */ + /** Comment-density ratio (0–1) that fails a file; `false`/`0` disables. Default 0.3. */ density?: number | false - /** Minimum added lines before density applies. Default 10. */ + /** Minimum added/scanned lines before density applies. Default 10. */ minAddedLines?: number } hardcodedColors?: { ignore?: string[]; root?: string } diff --git a/src/shared/diff.ts b/src/shared/diff.ts index a565394..1fe51a2 100644 --- a/src/shared/diff.ts +++ b/src/shared/diff.ts @@ -41,3 +41,26 @@ export function parseDiffAddedLines(diff: string): AddedLines { return added } + +/** Map each file in a unified diff to the content of the lines it removed (the `-` lines, marker stripped). */ +export function parseDiffRemovedLines(diff: string): Map { + const removed = new Map() + let currentFile: string | null = null + + for (const line of diff.split('\n')) { + const fileMatch = line.match(FILE_HEADER) + if (fileMatch) { + const file = fileMatch[1] as string + currentFile = file === '/dev/null' ? null : file + continue + } + if (line.startsWith('---') || HUNK_HEADER.test(line) || currentFile === null) continue + if (line.startsWith('-')) { + const arr = removed.get(currentFile) ?? [] + arr.push(line.slice(1)) + removed.set(currentFile, arr) + } + } + + return removed +} From cc6d16204287d9d322f285b42035ec3a34b2d0b1 Mon Sep 17 00:00:00 2001 From: "Rob Moore (MakerX)" Date: Thu, 9 Jul 2026 19:10:42 +0800 Subject: [PATCH 05/11] feat: monorepo-safe .claude resolution + scaffold comment rule file verifyx init now writes .claude/.agent-skills to the nearest existing .claude (cwd, else the closest ancestor within 3 levels, never past the git root, else cwd), with a --claude-dir override, and reports where it landed. Claude Code loads project settings only from its launch dir, so this attaches to the likely launch root instead of clobbering a monorepo parent. Also scaffolds a comments-only-when-non-obvious rule (keep/delete criteria + examples) alongside the skills. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../rules/comments-only-when-non-obvious.md | 80 +++++++++++++++++++ src/commands/registerInit.ts | 11 ++- src/scaffold/agentFiles.ts | 17 ++-- src/scaffold/claudeDir.test.ts | 53 ++++++++++++ src/scaffold/claudeDir.ts | 37 +++++++++ src/scaffold/init.ts | 14 +++- .../rules/comments-only-when-non-obvious.md | 80 +++++++++++++++++++ 7 files changed, 281 insertions(+), 11 deletions(-) create mode 100644 .claude/rules/comments-only-when-non-obvious.md create mode 100644 src/scaffold/claudeDir.test.ts create mode 100644 src/scaffold/claudeDir.ts create mode 100644 templates/rules/comments-only-when-non-obvious.md diff --git a/.claude/rules/comments-only-when-non-obvious.md b/.claude/rules/comments-only-when-non-obvious.md new file mode 100644 index 0000000..3413513 --- /dev/null +++ b/.claude/rules/comments-only-when-non-obvious.md @@ -0,0 +1,80 @@ +# Comment only when the code would mislead a competent reader + +Write self-documenting code: let well-named identifiers, narrow types, and small functions carry the meaning. +Add a comment only when reading the code alone would make a competent reader mispredict its behaviour, and only +to explain the _why_ the code itself cannot show. + +## When a comment earns its place + +Add one — kept to a single line where you can — for exactly these: + +- A non-obvious workaround: a specific bug, race, ordering hazard, or platform quirk the code is shaped around. +- A constraint imposed by an external system not visible in the code: an API contract, a wire format, a rate + limit, a required call order. +- A deliberate deviation from the obvious approach, where the obvious approach is wrong for a reason a reader + would not guess. + +**The keep-it test:** if a competent reader would predict the behaviour correctly without the comment, delete it. + +## What never to write + +- **Prose blocks on trivial code.** A one-liner, an assignment, or a single call never gets a multi-line comment. +- **Restatement.** A comment that re-says what the next line already says in code is noise; the code is the + source of truth, and a description of it rots and misleads. +- **Session narration.** Never write the editing session's reasoning into the file: `let me…`, `now I'll…`, `as +requested`, `this function does…`, `first,` / `next,`. That belongs in the chat, not the code. +- **LLM tells.** Em-dashes and curly quotes (`—`, `“ ” ‘ ’`) in a comment are a strong sign it was generated + narration rather than a deliberate note. +- **Duplicated explanation.** State a fact once, at its source of truth; a second site gets a one-line pointer, + not a copy. + +## Examples + +Restatement / prose block — delete it; the code already says this: + +```ts +// BAD +// Calculate the total by multiplying quantity by unit price, then store it so the +// invoice summary can use it later when it renders the line item. +const total = quantity * unitPrice + +// GOOD +const total = quantity * unitPrice +``` + +A justified comment — keep it; the constraint is external and invisible in the code: + +```ts +// GOOD +// context: Stripe rejects sub-cent precision — round to integer cents before sending. +const amountCents = Math.round(price * 100) +``` + +Session narration — strip it: + +```ts +// BAD +// Now let me add the retry logic that was requested. This wraps the call in a loop +// and backs off exponentially. +for (let attempt = 0; attempt < maxRetries; attempt++) { … } + +// GOOD +for (let attempt = 0; attempt < maxRetries; attempt++) { … } +``` + +## The two escape hatches + +Two things are always allowed and never need pruning: + +- **JSDoc** (`/** … */`). +- A comment whose first line starts with **`context:`** — the marker for durable context the code cannot + express. Use it only for a genuinely necessary _why_; it is not a way to keep a restatement. + +Machine directives (`eslint-disable`, `@ts-expect-error`, `prettier-ignore`, …) are exempt too. + +## Enforcement + +This is gated, not merely advised. `verifyx comments` flags low-value comments — long blocks, session narration, +and high comment density — over the current diff (or the whole codebase with `--scope all`). An edit-time +PostToolUse hook (`verifyx comments-hook`) applies the same rules the moment a file is edited. The +`prune-comments` skill strips and consolidates comments across a change on demand. diff --git a/src/commands/registerInit.ts b/src/commands/registerInit.ts index 67f50f3..395eeef 100644 --- a/src/commands/registerInit.ts +++ b/src/commands/registerInit.ts @@ -35,6 +35,7 @@ type InitCliOptions = { claude?: boolean agents?: boolean commentHook?: boolean + claudeDir?: string } type Selections = { checks: string[]; targets: AgentTarget[]; defaultsOnly: boolean; commentHook: boolean } @@ -92,6 +93,13 @@ function report(result: InitResult, defaultsOnly: boolean): void { if (file.action === 'unchanged') continue console.log(` ${ACTION_MARK[file.action]} ${file.path} (${file.action})`) } + if (result.rootDir !== process.cwd()) { + console.log( + color.yellow( + `\nAgent files were written to ${result.rootDir} (nearest existing .claude). Claude Code only loads them when launched from there — pass --claude-dir to target a different directory.`, + ), + ) + } } /** Summarise the dependency install: what was skipped, installed, and — last, so it's the takeaway — what failed. */ @@ -125,11 +133,12 @@ export function registerInit(program: Command): void { .option('--no-claude', 'do not write .claude/ files (non-interactive)') .option('--agents', 'also write .agent-skills/ files (non-interactive)') .option('--no-comment-hook', 'do not register the edit-time comment PostToolUse hook') + .option('--claude-dir ', 'directory to write .claude/.agent-skills into (default: nearest existing .claude, else cwd)') .action(async (opts: InitCliOptions) => { const cwd = process.cwd() const { checks, targets, defaultsOnly, commentHook } = await resolveSelections(opts) - const result = applyInit({ cwd, checks, targets, defaultsOnly, commentHook }) + const result = applyInit({ cwd, checks, targets, defaultsOnly, commentHook, claudeDir: opts.claudeDir }) report(result, defaultsOnly) if (result.devDeps.length > 0) { diff --git a/src/scaffold/agentFiles.ts b/src/scaffold/agentFiles.ts index 16463a9..38cab7c 100644 --- a/src/scaffold/agentFiles.ts +++ b/src/scaffold/agentFiles.ts @@ -26,21 +26,24 @@ function readTemplate(relativePath: string): string { * Skills are CLI-owned (created/updated as a whole). The instruction files are user-owned, so the pointer is * only appended when absent and existing content is never rewritten. */ -export function writeAgentFiles(cwd: string, targets: readonly AgentTarget[]): ManagedFileResult[] { +export function writeAgentFiles(rootDir: string, targets: readonly AgentTarget[]): ManagedFileResult[] { const results: ManagedFileResult[] = [] const skill = readTemplate('skills/verify/SKILL.md') const pruneSkill = readTemplate('skills/prune-comments/SKILL.md') + const commentRule = readTemplate('rules/comments-only-when-non-obvious.md') const guidance = readTemplate('verify-guidance.md') if (targets.includes('claude')) { - writeManaged(path.join(cwd, '.claude', 'skills', 'verify', 'SKILL.md'), skill, results) - writeManaged(path.join(cwd, '.claude', 'skills', 'prune-comments', 'SKILL.md'), pruneSkill, results) - ensurePointer(path.join(cwd, 'CLAUDE.md'), guidance, POINTER_MARKER, results) + writeManaged(path.join(rootDir, '.claude', 'skills', 'verify', 'SKILL.md'), skill, results) + writeManaged(path.join(rootDir, '.claude', 'skills', 'prune-comments', 'SKILL.md'), pruneSkill, results) + writeManaged(path.join(rootDir, '.claude', 'rules', 'comments-only-when-non-obvious.md'), commentRule, results) + ensurePointer(path.join(rootDir, 'CLAUDE.md'), guidance, POINTER_MARKER, results) } if (targets.includes('agents')) { - writeManaged(path.join(cwd, '.agent-skills', 'verify', 'SKILL.md'), skill, results) - writeManaged(path.join(cwd, '.agent-skills', 'prune-comments', 'SKILL.md'), pruneSkill, results) - ensurePointer(path.join(cwd, 'AGENTS.md'), guidance, POINTER_MARKER, results) + writeManaged(path.join(rootDir, '.agent-skills', 'verify', 'SKILL.md'), skill, results) + writeManaged(path.join(rootDir, '.agent-skills', 'prune-comments', 'SKILL.md'), pruneSkill, results) + writeManaged(path.join(rootDir, '.agent-skills', 'rules', 'comments-only-when-non-obvious.md'), commentRule, results) + ensurePointer(path.join(rootDir, 'AGENTS.md'), guidance, POINTER_MARKER, results) } results.sort((a, b) => a.path.localeCompare(b.path)) diff --git a/src/scaffold/claudeDir.test.ts b/src/scaffold/claudeDir.test.ts new file mode 100644 index 0000000..edbd251 --- /dev/null +++ b/src/scaffold/claudeDir.test.ts @@ -0,0 +1,53 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' + +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { resolveClaudeDir } from './claudeDir.ts' + +let dir: string +const mk = (...segs: string[]): string => { + const p = path.join(dir, ...segs) + fs.mkdirSync(p, { recursive: true }) + return p +} + +beforeEach(() => { + dir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'verify-claudedir-'))) +}) +afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }) +}) + +describe('resolveClaudeDir', () => { + it('uses cwd when it already has a .claude', () => { + const cwd = mk('pkg') + mk('pkg', '.claude') + expect(resolveClaudeDir(cwd)).toBe(cwd) + }) + + it('attaches to the nearest ancestor .claude within range', () => { + const root = mk('root') + mk('root', '.claude') + const cwd = mk('root', 'a', 'b') + expect(resolveClaudeDir(cwd)).toBe(root) + }) + + it('creates at cwd when no .claude is nearby', () => { + const cwd = mk('lonely') + expect(resolveClaudeDir(cwd)).toBe(cwd) + }) + + it('does not climb past the git root to an ancestor .claude', () => { + mk('.claude') // above the repo + mk('repo', '.git') + const cwd = mk('repo', 'pkg') + expect(resolveClaudeDir(cwd)).toBe(cwd) + }) + + it('honours an explicit override (resolved against cwd)', () => { + const cwd = mk('pkg') + expect(resolveClaudeDir(cwd, '../elsewhere')).toBe(path.resolve(cwd, '../elsewhere')) + }) +}) diff --git a/src/scaffold/claudeDir.ts b/src/scaffold/claudeDir.ts new file mode 100644 index 0000000..60c6b94 --- /dev/null +++ b/src/scaffold/claudeDir.ts @@ -0,0 +1,37 @@ +import fs from 'node:fs' +import path from 'node:path' + +const MAX_PARENTS = 3 + +function findGitRoot(from: string): string | null { + let dir = from + for (;;) { + if (fs.existsSync(path.join(dir, '.git'))) return dir + const parent = path.dirname(dir) + if (parent === dir) return null + dir = parent + } +} + +/** + * Resolve which directory init should write its `.claude` / `.agent-skills` integration into. Claude Code loads + * project settings only from the directory it is launched from (no inheritance), so the nearest existing + * `.claude` is the best available signal for that launch root: use `cwd` if it has one, else the closest ancestor + * within `MAX_PARENTS` (never climbing past the git root), else fall back to creating one at `cwd`. An explicit + * override always wins. + */ +export function resolveClaudeDir(cwd: string, override?: string): string { + if (override) return path.resolve(cwd, override) + if (fs.existsSync(path.join(cwd, '.claude'))) return cwd + + const gitRoot = findGitRoot(cwd) + let dir = cwd + for (let i = 0; i < MAX_PARENTS; i++) { + const parent = path.dirname(dir) + if (parent === dir) break + dir = parent + if (fs.existsSync(path.join(dir, '.claude'))) return dir + if (gitRoot && dir === gitRoot) break + } + return cwd +} diff --git a/src/scaffold/init.ts b/src/scaffold/init.ts index 9245dc9..0e80476 100644 --- a/src/scaffold/init.ts +++ b/src/scaffold/init.ts @@ -3,6 +3,7 @@ import path from 'node:path' import { getCheck } from '../checks/registry.ts' import type { Check } from '../checks/types.ts' import { type AgentTarget, writeAgentFiles } from './agentFiles.ts' +import { resolveClaudeDir } from './claudeDir.ts' import { ensureClaudeHook } from './claudeSettings.ts' import { ensureKnipIgnores } from './knipConfig.ts' import { addVerifyScripts } from './packageScripts.ts' @@ -17,6 +18,8 @@ export type InitOptions = { defaultsOnly: boolean /** Register the edit-time comment gate as a Claude PostToolUse hook (claude target only). Default off. */ commentHook?: boolean + /** Explicit directory to write `.claude`/`.agent-skills` into; overrides the nearest-`.claude` resolution. */ + claudeDir?: string } export type InitResult = { @@ -24,6 +27,8 @@ export type InitResult = { /** devDependencies the selected external checks need (deduped). */ devDeps: string[] agentFiles: ManagedFileResult[] + /** The directory the agent integration was written to (may differ from cwd in a monorepo). */ + rootDir: string } /** Pure scaffolding step: write package.json scripts + agent files, and report the devDeps to install. */ @@ -41,10 +46,13 @@ export function applyInit(opts: InitOptions): InitResult { // Defaults-only wires the top `verify` script to `verifyx all` so it runs every built-in with no verify:* list. const addedScripts = addVerifyScripts(path.join(opts.cwd, 'package.json'), scripts, opts.defaultsOnly ? 'verifyx all' : 'verifyx') - const agentFiles = writeAgentFiles(opts.cwd, opts.targets) + // context: Claude Code loads settings only from its launch dir, so we attach to the nearest existing .claude + // context: (the likely launch root), never a parent that isn't already a Claude project. See resolveClaudeDir. + const rootDir = resolveClaudeDir(opts.cwd, opts.claudeDir) + const agentFiles = writeAgentFiles(rootDir, opts.targets) // The PostToolUse hook is Claude-specific; other agents have no universal edit-time equivalent. - if (opts.commentHook && opts.targets.includes('claude')) ensureClaudeHook(opts.cwd, agentFiles) + if (opts.commentHook && opts.targets.includes('claude')) ensureClaudeHook(rootDir, agentFiles) // context: with unused-code selected, teach knip to ignore the other external tools verifyx runs at runtime. if (opts.checks.includes('unused-code')) { @@ -56,5 +64,5 @@ export function applyInit(opts: InitOptions): InitResult { ensureKnipIgnores(opts.cwd, [...new Set(toolDeps)], agentFiles) } - return { addedScripts, devDeps: [...new Set(devDeps)], agentFiles } + return { addedScripts, devDeps: [...new Set(devDeps)], agentFiles, rootDir } } diff --git a/templates/rules/comments-only-when-non-obvious.md b/templates/rules/comments-only-when-non-obvious.md new file mode 100644 index 0000000..3413513 --- /dev/null +++ b/templates/rules/comments-only-when-non-obvious.md @@ -0,0 +1,80 @@ +# Comment only when the code would mislead a competent reader + +Write self-documenting code: let well-named identifiers, narrow types, and small functions carry the meaning. +Add a comment only when reading the code alone would make a competent reader mispredict its behaviour, and only +to explain the _why_ the code itself cannot show. + +## When a comment earns its place + +Add one — kept to a single line where you can — for exactly these: + +- A non-obvious workaround: a specific bug, race, ordering hazard, or platform quirk the code is shaped around. +- A constraint imposed by an external system not visible in the code: an API contract, a wire format, a rate + limit, a required call order. +- A deliberate deviation from the obvious approach, where the obvious approach is wrong for a reason a reader + would not guess. + +**The keep-it test:** if a competent reader would predict the behaviour correctly without the comment, delete it. + +## What never to write + +- **Prose blocks on trivial code.** A one-liner, an assignment, or a single call never gets a multi-line comment. +- **Restatement.** A comment that re-says what the next line already says in code is noise; the code is the + source of truth, and a description of it rots and misleads. +- **Session narration.** Never write the editing session's reasoning into the file: `let me…`, `now I'll…`, `as +requested`, `this function does…`, `first,` / `next,`. That belongs in the chat, not the code. +- **LLM tells.** Em-dashes and curly quotes (`—`, `“ ” ‘ ’`) in a comment are a strong sign it was generated + narration rather than a deliberate note. +- **Duplicated explanation.** State a fact once, at its source of truth; a second site gets a one-line pointer, + not a copy. + +## Examples + +Restatement / prose block — delete it; the code already says this: + +```ts +// BAD +// Calculate the total by multiplying quantity by unit price, then store it so the +// invoice summary can use it later when it renders the line item. +const total = quantity * unitPrice + +// GOOD +const total = quantity * unitPrice +``` + +A justified comment — keep it; the constraint is external and invisible in the code: + +```ts +// GOOD +// context: Stripe rejects sub-cent precision — round to integer cents before sending. +const amountCents = Math.round(price * 100) +``` + +Session narration — strip it: + +```ts +// BAD +// Now let me add the retry logic that was requested. This wraps the call in a loop +// and backs off exponentially. +for (let attempt = 0; attempt < maxRetries; attempt++) { … } + +// GOOD +for (let attempt = 0; attempt < maxRetries; attempt++) { … } +``` + +## The two escape hatches + +Two things are always allowed and never need pruning: + +- **JSDoc** (`/** … */`). +- A comment whose first line starts with **`context:`** — the marker for durable context the code cannot + express. Use it only for a genuinely necessary _why_; it is not a way to keep a restatement. + +Machine directives (`eslint-disable`, `@ts-expect-error`, `prettier-ignore`, …) are exempt too. + +## Enforcement + +This is gated, not merely advised. `verifyx comments` flags low-value comments — long blocks, session narration, +and high comment density — over the current diff (or the whole codebase with `--scope all`). An edit-time +PostToolUse hook (`verifyx comments-hook`) applies the same rules the moment a file is edited. The +`prune-comments` skill strips and consolidates comments across a change on demand. From 6adfdd7f5c2c52541f6c644c4304d510da430f51 Mon Sep 17 00:00:00 2001 From: "Rob Moore (MakerX)" Date: Thu, 9 Jul 2026 19:10:42 +0800 Subject: [PATCH 06/11] docs: scope-aware prune skill + document scope/block-all and monorepo behaviour Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/skills/prune-comments/SKILL.md | 64 +++++++++++------------- README.md | 44 ++++++++++------ templates/skills/prune-comments/SKILL.md | 64 +++++++++++------------- 3 files changed, 85 insertions(+), 87 deletions(-) diff --git a/.claude/skills/prune-comments/SKILL.md b/.claude/skills/prune-comments/SKILL.md index b268a92..7f951e3 100644 --- a/.claude/skills/prune-comments/SKILL.md +++ b/.claude/skills/prune-comments/SKILL.md @@ -1,57 +1,49 @@ --- name: prune-comments -description: Remove low-value code comments across the current change. Use when asked to "prune comments", "clean up comments", or after the verify comments check flags narration, dense comments, or long comment blocks. +description: Strip low-value comments (restatement, session narration, prose blocks, dense runs) from a change. Applies edits in-place, file by file. Use when asked to "prune comments", "clean up comments", "remove comment bloat", or after the verify comments check flags them. Apply-capable — it edits files, it is not a read-only review. --- -Capable coding agents love to narrate their work in comments. This skill removes that noise -from the current change while protecting the comments that genuinely earn their place. +# prune-comments + +Remove comment bloat and keep only the comments that earn their place, following the criteria in the +`comments-only-when-non-obvious` rule (`.claude/rules/` / `.agent-skills/rules/`). Edits are applied in-place. ## 1. Find the flagged comments -Run the comments check over the branch diff (the binary is `verifyx`, since `verify` is a -Windows builtin): +Run the check to see what fails (the binary is `verifyx`). By default it judges the **current diff** — the +comments this change introduced: ``` -npx verifyx comments --block-new-comments --pushback +npx verifyx comments --pushback ``` -That lists every comment on a changed line. For a softer pass that only flags narration and -dense comment runs (the defaults), drop `--block-new-comments`. - -## 2. Delete or keep — the criteria +Scope options (`verifyx comments` handles the git/diff scoping itself — you don't need to compute it): -**Delete** a comment when any of these hold: +- **Whole repo:** `npx verifyx comments --scope all --pushback` — audit and prune every comment in the codebase, + not just the diff. +- **Every comment, not just heuristic hits:** add `--block-all` (with either scope) when the goal is to remove + _all_ comments in scope, keeping only `context:`/JSDoc. -- It narrates _what_ the next line does (`// increment the counter`, `// return the result`). -- It narrates the editing session (`// let me add a handler`, `// as requested`, `// now we…`). -- It restates a name already in the code (`// user service` above `class UserService`). -- It is a commented-out block of old code. -- Removing it loses nothing a competent reader couldn't recover from the code itself. +## 2. Delete or keep -**Keep** a comment when it explains _why_, not _what_: +Apply the `comments-only-when-non-obvious` rule to each flagged comment: -- A non-obvious constraint, workaround, or business rule (`// Stripe rejects amounts under 50c`). -- A deliberate deviation a reader would otherwise "fix" and break. -- A link to an issue/spec that explains a surprising choice. +- **Delete** restatement, session narration (`let me…`, `as requested`, `this function does…`), prose blocks on + trivial code, LLM tells (em-dash / curly quotes), and duplicated explanation. When in doubt, delete. +- **Keep** only a non-obvious _why_: a workaround, an external constraint, or a deliberate non-obvious deviation. -If a kept comment is genuinely durable context the code cannot express, prefix its first line -with `context:` so the check leaves it alone: - -```ts -// context: retries must stay at 3 — the upstream gateway drops the connection after the 4th. -``` +Prefer fixing the code over keeping the comment: a better name, a smaller function, or a narrower type often +removes the need for the comment entirely. -Machine directives (`eslint-disable`, `@ts-expect-error`, …) and JSDoc (`/** … */`) are -always allowed and never need pruning. +If a kept comment is genuinely durable context the code cannot express, prefix its first line with `context:`. +Do **not** mark low-value comments `context:` to slip them past the gate — `context:` pages a human to approve it. -## 3. Prefer fixing the code over keeping the comment +## 3. Apply and re-run -When a comment exists because the code is unclear, the better fix is usually to rename a -variable, extract a well-named function, or simplify — then delete the comment. Reach for -`context:` only when the code truly cannot carry the meaning. +Edit with the Edit tool — one edit per change site, not a whole-file rewrite. Then re-run the same +`verifyx comments` command and repeat until it passes. Never silence the check or widen its ignore globs. -## 4. Re-run until clean +## Output -After editing, run the check again and repeat until it passes. Do not silence the check, -widen its ignore globs, or mark low-value comments `context:` to slip them past the gate — -`context:` pages a human to approve the comment. +One line per file: `pruned: — removed`, `clean: `, or `needs manual review: `. +No preamble, no closing remarks. diff --git a/README.md b/README.md index cf6b458..3b55dc8 100644 --- a/README.md +++ b/README.md @@ -170,28 +170,33 @@ verifyx comments --max-lines 2 --pushback "src/**/*.ts" Capable coding models (Opus 4.8 very much included) love to narrate their work: multi-line comment blocks explaining _what_ the code does rather than _why_. They add little value, drift out of sync as the code changes (and a later reader, human or LLM, may trust the stale comment over the code), and quietly grow the surface area you have to maintain. By default this check pushes back on exactly that, flagging any comment block taller than `--max-lines` (default 2) so the pressure is on the code to document itself. -- `[pattern]`: glob, directory, or file to scan. +The check has two orthogonal axes. **Scope** decides which comments are candidates — `diff` (only comments on changed lines; the default) or `all` (every comment in the matched files). **Strictness** decides what fails within that scope — the heuristics below (default), or `--block-all`, which fails _every_ comment in scope. + +- `[pattern]`: glob, directory, or file to scan (used by `--scope all`). +- `--scope `: judge only changed lines (default) or the whole codebase. - `--max-lines `: fail on comment blocks longer than `n` lines (default 2). - `--pushback`: reframe the failure message as back-pressure aimed at an AI agent (see below). - `--warn`: report the long-block violations without failing the run. - `--no-narration`: turn off the session-narration gate (on by default; see below). -- `--comment-density `: the changed-line comment share (0–1) that fails a file (default `0.3`; `0` disables). -- `--min-added-lines `: skip the density gate on diffs smaller than `n` added lines (default 10). -- `--block-new-comments`: fail on _any_ comment on a changed line — the strictest gate; supersedes narration/density (vs `HEAD` locally, the PR base in CI; see below). -- `--ignore `: exclude files (repeatable; the diff-scoped gates also read `comments.ignore` from your [verify config](#configuration)). +- `--comment-density `: the comment share (0–1) that fails a file (default `0.3`; `0` disables). +- `--min-added-lines `: skip the density gate when fewer than `n` lines are in scope (default 10). +- `--block-all`: fail _every_ non-exempt comment in scope — the strictest gate; supersedes the heuristics. +- `--block-new-comments`: alias for `--scope diff --block-all` (fail every comment on a changed line). +- `--ignore `: exclude files (repeatable; also read from `comments.ignore` in your [verify config](#configuration)). -Beyond the long-block check, two **diff-scoped heuristics** run on changed lines by default (so they judge only what a change introduces, never legacy comments): +Three heuristics run by default within the scope: -- **Narration** flags comments that think out loud or restate _what_ the next line does — `// let me add the handler`, `// as requested`, `// this function returns the total`. These are the clearest tell that an agent is narrating its edit rather than documenting a decision. -- **Density** fails a changed file when comments make up too large a share of its added lines (default 30%, on diffs of 10+ added lines). Dense comment runs almost always narrate rather than explain. +- **Long blocks** — a run of comment lines (or a `/* … */` block) taller than `--max-lines`. +- **Narration** — comments that think out loud or restate _what_ the next line does (`// let me add the handler`, `// as requested`, `// this function returns the total`), plus LLM punctuation tells (em-dashes and curly quotes, which generated comments emit far more than hand-typed ones). +- **Density** — a file where comments make up too large a share of the lines in scope (default 30%, once at least 10 lines are in scope). A change that _removes_ at least as many comment lines as it adds is never flagged — a trim can't be adding bloat. -Both honour the same escape hatches as everything else (JSDoc, `context:`, machine directives). Tune or disable them per repo via [verify config](#configuration). +The default `diff` scope means the heuristics judge only what a change introduces, never legacy comments — so you can adopt the check on an existing repo without a cleanup first. Use `--scope all` for a full-codebase audit. All gates honour the same escape hatches (JSDoc, `context:`, machine directives); tune or disable them per repo via [verify config](#configuration). **`--pushback` is the clever bit.** An AI agent that hits a failing check will often take the path of least resistance and just delete or weaken the check to make the run pass. So rather than a dry error, the pushback message tells the agent that the _only_ sanctioned way to keep the comment is to prefix it with `context:`, and that doing so **pages a human to approve it**. Confronted with a real person's time on the line, the agent tends to reconsider and remove the low-value comment instead of gaming the gate. It is a small piece of prompt-engineering baked into a lint failure, and in practice it stops agents silencing the check far more reliably than a plain error does. -If you want to go further and block **new** comments outright, override the `verify:comments` script and add `--block-new-comments`. That turns on a stricter, diff-based gate on top of the long-block check: any comment sitting on a changed line fails, whether you added or merely edited it. Machine directives (`eslint-disable`, `@ts-expect-error`, and friends) stay exempt, as does anything marked `context:`. +If you want to go further and block comments outright, add `--block-all`: every non-exempt comment in scope fails, not just the heuristic hits. Pair it with the scope you want — `--scope diff --block-all` (the `--block-new-comments` alias) fails any comment on a changed line, while `--scope all --block-all` is a zero-comment policy across the codebase. Machine directives (`eslint-disable`, `@ts-expect-error`, and friends) stay exempt, as does anything marked `context:`. -The "changed lines" are resolved per environment so the gate works the same locally and in CI. Locally it diffs the working tree against `HEAD` (your uncommitted changes). In CI (`CI` set) a clean checkout has nothing uncommitted, so it diffs against the **merge base with the PR base branch** instead, read from `GITHUB_BASE_REF` (GitHub Actions). Set `VERIFY_DIFF_BASE` to a ref to override the base explicitly; if no base can be resolved it falls back to `HEAD`. For CI, make sure the base branch is fetched (`actions/checkout` with `fetch-depth: 0`), or the merge base won't be found and the gate silently passes. +Under the default `diff` scope, the "changed lines" are resolved per environment so the gate works the same locally and in CI. Locally it diffs the working tree against `HEAD` (your uncommitted changes). In CI (`CI` set) a clean checkout has nothing uncommitted, so it diffs against the **merge base with the PR base branch** instead, read from `GITHUB_BASE_REF` (GitHub Actions). Set `VERIFY_DIFF_BASE` to a ref to override the base explicitly; if no base can be resolved it falls back to `HEAD`. For CI, make sure the base branch is fetched (`actions/checkout` with `fetch-depth: 0`), or the merge base won't be found and the gate silently passes. Two escape hatches keep genuinely useful comments alive. **JSDoc** (`/** … */`) is always allowed, and prefixing a comment's first line with `context:` marks it as durable context the code itself can't express: @@ -213,11 +218,13 @@ The `comments` check runs at verify/CI time — the end of a task. To close the } ``` -`verifyx comments-hook` reads the tool payload on stdin, scans only the text that edit introduced (no git needed), and on a long block, narration, or dense comments writes back-pressure to stderr and exits `2` — which Claude surfaces to the agent in the same turn, so it revises before the code ever reaches you. The hook is Claude-specific (other agents have no universal edit-time equivalent); those rely on the verify/CI check and the `prune-comments` skill. +`verifyx comments-hook` reads the tool payload on stdin, scans only the text that edit introduced (no git needed), and on a long block, narration, or dense comments writes back-pressure to stderr and exits `2` — which Claude surfaces to the agent in the same turn, so it revises before the code ever reaches you. The hook is always diff-scoped (it only sees the edit), and Claude-specific (other agents have no universal edit-time equivalent); those rely on the verify/CI check and the `prune-comments` skill. + +> **Monorepos.** Claude Code loads `.claude/settings.json` only from the directory it is _launched_ from — it does not walk up or inherit. `verifyx init` writes the hook (and the skills, rules, and `CLAUDE.md` pointer) to the **nearest existing `.claude`** — your cwd if it has one, otherwise the closest ancestor within a few levels, never past the git root — and prints where it landed. Run `init` from (or point `--claude-dir` at) the directory you launch Claude from, so the hook actually fires. -#### `prune-comments` skill +#### `prune-comments` skill + comment rule -`verifyx init` also drops a **`prune-comments`** skill (`.claude/skills` + `.agent-skills`) — a remediation companion to the gate. Ask an agent to "prune comments" and it runs the check over the branch diff, then walks the delete/keep criteria (with `context:` as the sanctioned escape hatch) to clean up low-value comments across an existing change. +`verifyx init` also drops a **`prune-comments`** skill and a **`comments-only-when-non-obvious`** rule (`.claude/rules` + `.agent-skills/rules`) — the remediation companion to the gate, with explicit keep/delete criteria and worked examples. Ask an agent to "prune comments" and it runs `verifyx comments` over the diff, then applies the rule to clean up low-value comments (with `context:` as the sanctioned escape hatch). To sweep the **whole codebase** rather than just a change, it runs `verifyx comments --scope all`. ### `hardcoded-colors` @@ -300,7 +307,14 @@ The **native** checks that take persistent settings read them from a `verify.con ```jsonc { "verify": { - "comments": { "ignore": ["**/*.generated.ts"], "narration": true, "density": 0.3, "minAddedLines": 10 }, + "comments": { + "ignore": ["**/*.generated.ts"], + "scope": "diff", + "blockAll": false, + "narration": true, + "density": 0.3, + "minAddedLines": 10, + }, "hardcodedColors": { "root": "src", "ignore": ["**/tokens.ts"] }, "forbiddenStrings": [{ "file": "app.json", "paths": ["env.LOG_LEVEL"], "disallowed": "debug" }], }, diff --git a/templates/skills/prune-comments/SKILL.md b/templates/skills/prune-comments/SKILL.md index b268a92..7f951e3 100644 --- a/templates/skills/prune-comments/SKILL.md +++ b/templates/skills/prune-comments/SKILL.md @@ -1,57 +1,49 @@ --- name: prune-comments -description: Remove low-value code comments across the current change. Use when asked to "prune comments", "clean up comments", or after the verify comments check flags narration, dense comments, or long comment blocks. +description: Strip low-value comments (restatement, session narration, prose blocks, dense runs) from a change. Applies edits in-place, file by file. Use when asked to "prune comments", "clean up comments", "remove comment bloat", or after the verify comments check flags them. Apply-capable — it edits files, it is not a read-only review. --- -Capable coding agents love to narrate their work in comments. This skill removes that noise -from the current change while protecting the comments that genuinely earn their place. +# prune-comments + +Remove comment bloat and keep only the comments that earn their place, following the criteria in the +`comments-only-when-non-obvious` rule (`.claude/rules/` / `.agent-skills/rules/`). Edits are applied in-place. ## 1. Find the flagged comments -Run the comments check over the branch diff (the binary is `verifyx`, since `verify` is a -Windows builtin): +Run the check to see what fails (the binary is `verifyx`). By default it judges the **current diff** — the +comments this change introduced: ``` -npx verifyx comments --block-new-comments --pushback +npx verifyx comments --pushback ``` -That lists every comment on a changed line. For a softer pass that only flags narration and -dense comment runs (the defaults), drop `--block-new-comments`. - -## 2. Delete or keep — the criteria +Scope options (`verifyx comments` handles the git/diff scoping itself — you don't need to compute it): -**Delete** a comment when any of these hold: +- **Whole repo:** `npx verifyx comments --scope all --pushback` — audit and prune every comment in the codebase, + not just the diff. +- **Every comment, not just heuristic hits:** add `--block-all` (with either scope) when the goal is to remove + _all_ comments in scope, keeping only `context:`/JSDoc. -- It narrates _what_ the next line does (`// increment the counter`, `// return the result`). -- It narrates the editing session (`// let me add a handler`, `// as requested`, `// now we…`). -- It restates a name already in the code (`// user service` above `class UserService`). -- It is a commented-out block of old code. -- Removing it loses nothing a competent reader couldn't recover from the code itself. +## 2. Delete or keep -**Keep** a comment when it explains _why_, not _what_: +Apply the `comments-only-when-non-obvious` rule to each flagged comment: -- A non-obvious constraint, workaround, or business rule (`// Stripe rejects amounts under 50c`). -- A deliberate deviation a reader would otherwise "fix" and break. -- A link to an issue/spec that explains a surprising choice. +- **Delete** restatement, session narration (`let me…`, `as requested`, `this function does…`), prose blocks on + trivial code, LLM tells (em-dash / curly quotes), and duplicated explanation. When in doubt, delete. +- **Keep** only a non-obvious _why_: a workaround, an external constraint, or a deliberate non-obvious deviation. -If a kept comment is genuinely durable context the code cannot express, prefix its first line -with `context:` so the check leaves it alone: - -```ts -// context: retries must stay at 3 — the upstream gateway drops the connection after the 4th. -``` +Prefer fixing the code over keeping the comment: a better name, a smaller function, or a narrower type often +removes the need for the comment entirely. -Machine directives (`eslint-disable`, `@ts-expect-error`, …) and JSDoc (`/** … */`) are -always allowed and never need pruning. +If a kept comment is genuinely durable context the code cannot express, prefix its first line with `context:`. +Do **not** mark low-value comments `context:` to slip them past the gate — `context:` pages a human to approve it. -## 3. Prefer fixing the code over keeping the comment +## 3. Apply and re-run -When a comment exists because the code is unclear, the better fix is usually to rename a -variable, extract a well-named function, or simplify — then delete the comment. Reach for -`context:` only when the code truly cannot carry the meaning. +Edit with the Edit tool — one edit per change site, not a whole-file rewrite. Then re-run the same +`verifyx comments` command and repeat until it passes. Never silence the check or widen its ignore globs. -## 4. Re-run until clean +## Output -After editing, run the check again and repeat until it passes. Do not silence the check, -widen its ignore globs, or mark low-value comments `context:` to slip them past the gate — -`context:` pages a human to approve the comment. +One line per file: `pruned: — removed`, `clean: `, or `needs manual review: `. +No preamble, no closing remarks. From 600603900dfd7f2bdf1f5bc7bc6f7ccd1f4a6faf Mon Sep 17 00:00:00 2001 From: "Rob Moore (MakerX)" Date: Thu, 9 Jul 2026 23:50:07 +0800 Subject: [PATCH 07/11] feat: init prompts for comment scope + strictness, baked into verify:comments verifyx init now asks (or takes --comment-scope diff|all and --comment-block-all) how the comments check should gate, and emits the flags into the scaffolded verify:comments script. Non-default choices are emitted even under defaults-only as a verify:comments override, since there's nowhere else to record them. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/commands/registerInit.ts | 50 ++++++++++++++++++++++++++++++++---- src/scaffold/init.test.ts | 16 ++++++++++++ src/scaffold/init.ts | 20 ++++++++++++++- 3 files changed, 80 insertions(+), 6 deletions(-) diff --git a/src/commands/registerInit.ts b/src/commands/registerInit.ts index 395eeef..816d5fb 100644 --- a/src/commands/registerInit.ts +++ b/src/commands/registerInit.ts @@ -36,9 +36,32 @@ type InitCliOptions = { agents?: boolean commentHook?: boolean claudeDir?: string + commentScope?: string + commentBlockAll?: boolean } -type Selections = { checks: string[]; targets: AgentTarget[]; defaultsOnly: boolean; commentHook: boolean } +type CommentScope = 'diff' | 'all' +type Selections = { + checks: string[] + targets: AgentTarget[] + defaultsOnly: boolean + commentHook: boolean + commentScope: CommentScope + commentBlockAll: boolean +} + +async function askCommentOptions(): Promise<{ commentScope: CommentScope; commentBlockAll: boolean }> { + const commentScope = (await ask('select', 'Comments — which comments should the check gate?', [ + { name: 'diff', message: 'Changed lines only (gate new code, skip legacy)', enabled: true }, + { name: 'all', message: 'The whole codebase' }, + ])) as CommentScope + const commentBlockAll = + (await ask('select', 'Comments — how strict?', [ + { name: 'heuristics', message: 'Heuristics: long blocks, narration, density', enabled: true }, + { name: 'blockAll', message: 'Block every comment in scope (context:/JSDoc still allowed)' }, + ])) === 'blockAll' + return { commentScope, commentBlockAll } +} async function resolveSelections(opts: InitCliOptions): Promise { const nonInteractive = !!opts.yes || !process.stdin.isTTY @@ -51,6 +74,8 @@ async function resolveSelections(opts: InitCliOptions): Promise { targets, defaultsOnly: !!opts.defaultsOnly, commentHook: opts.commentHook !== false, + commentScope: opts.commentScope === 'all' ? 'all' : 'diff', + commentBlockAll: !!opts.commentBlockAll, } } @@ -81,7 +106,11 @@ async function resolveSelections(opts: InitCliOptions): Promise { { name: 'no', message: 'No — rely on the verify/CI comments check only' }, ])) === 'yes' : false - return { checks, targets, defaultsOnly, commentHook } + + const { commentScope, commentBlockAll } = checks.includes('comments') + ? await askCommentOptions() + : { commentScope: 'diff' as CommentScope, commentBlockAll: false } + return { checks, targets, defaultsOnly, commentHook, commentScope, commentBlockAll } } function report(result: InitResult, defaultsOnly: boolean): void { @@ -134,11 +163,22 @@ export function registerInit(program: Command): void { .option('--agents', 'also write .agent-skills/ files (non-interactive)') .option('--no-comment-hook', 'do not register the edit-time comment PostToolUse hook') .option('--claude-dir ', 'directory to write .claude/.agent-skills into (default: nearest existing .claude, else cwd)') + .option('--comment-scope ', 'comments check scope baked into verify:comments: diff (default) or all') + .option('--comment-block-all', 'bake --block-all into verify:comments (fail every comment in scope)') .action(async (opts: InitCliOptions) => { const cwd = process.cwd() - const { checks, targets, defaultsOnly, commentHook } = await resolveSelections(opts) - - const result = applyInit({ cwd, checks, targets, defaultsOnly, commentHook, claudeDir: opts.claudeDir }) + const { checks, targets, defaultsOnly, commentHook, commentScope, commentBlockAll } = await resolveSelections(opts) + + const result = applyInit({ + cwd, + checks, + targets, + defaultsOnly, + commentHook, + claudeDir: opts.claudeDir, + commentScope, + commentBlockAll, + }) report(result, defaultsOnly) if (result.devDeps.length > 0) { diff --git a/src/scaffold/init.test.ts b/src/scaffold/init.test.ts index 84cea28..de3ab6d 100644 --- a/src/scaffold/init.test.ts +++ b/src/scaffold/init.test.ts @@ -56,6 +56,22 @@ describe('applyInit', () => { expect(result.devDeps).toEqual(expect.arrayContaining(['knip', 'jscpd', 'skott'])) }) + it('bakes the chosen comment scope/strictness into verify:comments', () => { + applyInit({ cwd: dir, checks: ['comments'], targets: [], defaultsOnly: false, commentScope: 'all', commentBlockAll: true }) + expect(readScripts()['verify:comments']).toBe('verifyx comments --pushback --scope all --block-all') + }) + + it('defaults comment options to a plain pushback script', () => { + applyInit({ cwd: dir, checks: ['comments'], targets: [], defaultsOnly: false }) + expect(readScripts()['verify:comments']).toBe('verifyx comments --pushback') + }) + + it('emits a verify:comments override under defaults-only only when a non-default comment option is chosen', () => { + applyInit({ cwd: dir, checks: CHECKS.map((c) => c.name), targets: [], defaultsOnly: true, commentBlockAll: true }) + expect(readScripts()['verify:comments']).toBe('verifyx comments --pushback --block-all') + expect(readScripts().verify).toBe('verifyx all') + }) + it('does not clobber an existing verify:* script', () => { fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ scripts: { 'verify:complexity': 'custom' } })) applyInit({ cwd: dir, checks: ['complexity'], targets: [], defaultsOnly: false }) diff --git a/src/scaffold/init.ts b/src/scaffold/init.ts index 0e80476..7074a48 100644 --- a/src/scaffold/init.ts +++ b/src/scaffold/init.ts @@ -20,6 +20,10 @@ export type InitOptions = { commentHook?: boolean /** Explicit directory to write `.claude`/`.agent-skills` into; overrides the nearest-`.claude` resolution. */ claudeDir?: string + /** Comments check scope baked into the scaffolded script. Default `diff`. */ + commentScope?: 'diff' | 'all' + /** Bake `--block-all` into the scaffolded comments script (fail every comment in scope). Default false. */ + commentBlockAll?: boolean } export type InitResult = { @@ -31,6 +35,14 @@ export type InitResult = { rootDir: string } +/** The scaffolded `verify:comments` command, with the chosen scope/strictness baked in as flags. */ +function commentsScript(opts: InitOptions): string { + const flags = ['--pushback'] + if (opts.commentScope === 'all') flags.push('--scope all') + if (opts.commentBlockAll) flags.push('--block-all') + return `verifyx comments ${flags.join(' ')}` +} + /** Pure scaffolding step: write package.json scripts + agent files, and report the devDeps to install. */ export function applyInit(opts: InitOptions): InitResult { const devDeps: string[] = [] @@ -40,7 +52,13 @@ export function applyInit(opts: InitOptions): InitResult { const check = getCheck(name) if (!check) continue if (check.scaffold.devDeps) devDeps.push(...check.scaffold.devDeps) - if (!opts.defaultsOnly) scripts[`verify:${name}`] = check.scaffold.script + if (!opts.defaultsOnly) scripts[`verify:${name}`] = name === 'comments' ? commentsScript(opts) : check.scaffold.script + } + + // context: even under defaults-only (no verify:* scripts) a non-default comment choice is emitted as a + // context: verify:comments override so `verifyx all` honours it; there's nowhere else to record the flags. + if (opts.defaultsOnly && opts.checks.includes('comments') && (opts.commentScope === 'all' || opts.commentBlockAll)) { + scripts['verify:comments'] = commentsScript(opts) } // Defaults-only wires the top `verify` script to `verifyx all` so it runs every built-in with no verify:* list. From 559d82b869d293a7c38a0b87f4174660a222ea44 Mon Sep 17 00:00:00 2001 From: "Rob Moore (MakerX)" Date: Sat, 11 Jul 2026 01:07:56 +0800 Subject: [PATCH 08/11] docs: restore full comment rule from claude-comment-gate; drop context: guidance Scopes the rule's path frontmatter to our extensions (ts/js/yml family); restores the content that was trimmed (the failure-mode preamble, the invented-shorthand and config/infra and duplicated-explanation entries) in TS/YAML; removes all context: guidance from the rule and the prune skill so agents meet the human-paging pushback at the gate rather than pre-learning the escape; no escape-hatch section. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../rules/comments-only-when-non-obvious.md | 79 ++++++++++--------- .claude/skills/prune-comments/SKILL.md | 10 +-- .../rules/comments-only-when-non-obvious.md | 79 ++++++++++--------- templates/skills/prune-comments/SKILL.md | 10 +-- 4 files changed, 94 insertions(+), 84 deletions(-) diff --git a/.claude/rules/comments-only-when-non-obvious.md b/.claude/rules/comments-only-when-non-obvious.md index 3413513..d57b9e6 100644 --- a/.claude/rules/comments-only-when-non-obvious.md +++ b/.claude/rules/comments-only-when-non-obvious.md @@ -1,36 +1,37 @@ +--- +paths: + - '*.{ts,tsx,cts,mts,js,jsx,mjs,cjs,yml,yaml}' + - '**/*.{ts,tsx,cts,mts,js,jsx,mjs,cjs,yml,yaml}' +--- + # Comment only when the code would mislead a competent reader -Write self-documenting code: let well-named identifiers, narrow types, and small functions carry the meaning. -Add a comment only when reading the code alone would make a competent reader mispredict its behaviour, and only -to explain the _why_ the code itself cannot show. +Write self-documenting code: let well-named identifiers, narrow types, and small functions carry the meaning. Add a comment only when reading the code alone would make a competent reader mispredict its behaviour, and only to explain the _why_ the code itself cannot show. ## When a comment earns its place -Add one — kept to a single line where you can — for exactly these: +Add one — kept to a single line where possible — for exactly these: - A non-obvious workaround: a specific bug, race, ordering hazard, or platform quirk the code is shaped around. -- A constraint imposed by an external system not visible in the code: an API contract, a wire format, a rate - limit, a required call order. -- A deliberate deviation from the obvious approach, where the obvious approach is wrong for a reason a reader - would not guess. +- A constraint imposed by an external system not visible in the code: an API contract, a wire format, a rate limit, a required call order. +- A deliberate deviation from the obvious approach, where the obvious approach is wrong for a reason a reader would not guess. -**The keep-it test:** if a competent reader would predict the behaviour correctly without the comment, delete it. +The keep-it test: if a competent reader would predict the behaviour correctly without the comment, delete the comment. ## What never to write -- **Prose blocks on trivial code.** A one-liner, an assignment, or a single call never gets a multi-line comment. -- **Restatement.** A comment that re-says what the next line already says in code is noise; the code is the - source of truth, and a description of it rots and misleads. -- **Session narration.** Never write the editing session's reasoning into the file: `let me…`, `now I'll…`, `as -requested`, `this function does…`, `first,` / `next,`. That belongs in the chat, not the code. -- **LLM tells.** Em-dashes and curly quotes (`—`, `“ ” ‘ ’`) in a comment are a strong sign it was generated - narration rather than a deliberate note. -- **Duplicated explanation.** State a fact once, at its source of truth; a second site gets a one-line pointer, - not a copy. +These are the failure modes this rule exists to stop. They are the current default-model drift, and they poison codebases at scale. + +- **Prose blocks on trivial code.** A one-liner, an assignment, or a single call never gets a multi-line comment. A paragraph above one statement is the dominant failure. +- **Restatement.** A comment that re-says what the next line already says in code is noise, not signal. The code is the source of truth; a description of it rots and must then be re-verified by every reader. +- **Leaked session narration.** Never write the editing session's ephemeral reasoning into code or to disk: `Let me…`, `Now I'll…`, `This is a substantial change…`, `we need to…`, `as requested`. That belongs in the chat, never the file. +- **LLM tells.** Em-dashes and curly quotes (`—`, `“ ” ‘ ’`) in a comment are a strong sign it was generated narration rather than a deliberate note; the gate flags them. +- **Invented shorthand.** Do not coin a new term, abbreviation, or naming scheme inside a comment and leave it undefined; it snowballs and is never cleaned up. Use the codebase's existing vocabulary. +- **Duplicated explanation.** State a fact once, at its source of truth (the README, or the single function that owns it). A second site gets a one-line pointer, not a copy — the same explanation restated across many files is the same bug as restatement, spread out. ## Examples -Restatement / prose block — delete it; the code already says this: +Restatement and a prose block — delete it; the code already says this: ```ts // BAD @@ -45,36 +46,42 @@ const total = quantity * unitPrice A justified comment — keep it; the constraint is external and invisible in the code: ```ts -// GOOD -// context: Stripe rejects sub-cent precision — round to integer cents before sending. +// GOOD: Stripe rejects sub-cent precision; round to integer cents before sending. const amountCents = Math.round(price * 100) ``` -Session narration — strip it: +Leaked session narration — strip it: ```ts // BAD -// Now let me add the retry logic that was requested. This wraps the call in a loop -// and backs off exponentially. -for (let attempt = 0; attempt < maxRetries; attempt++) { … } +// Now let me add the retry logic that was requested. This is the substantial +// part of the change: wrap the call in a loop and back off exponentially. +for (let attempt = 0; attempt < maxRetries; attempt++) { ... } // GOOD -for (let attempt = 0; attempt < maxRetries; attempt++) { … } +for (let attempt = 0; attempt < maxRetries; attempt++) { ... } ``` -## The two escape hatches +Config and infra are not exempt — the same restatement shows up in YAML: -Two things are always allowed and never need pruning: +```yaml +# BAD: the key and its default already say this +# The log level for the service, defaulting to info. +logLevel: info -- **JSDoc** (`/** … */`). -- A comment whose first line starts with **`context:`** — the marker for durable context the code cannot - express. Use it only for a genuinely necessary _why_; it is not a way to keep a restatement. +# GOOD +logLevel: info +``` + +Duplicated explanation — say it once, point to it: -Machine directives (`eslint-disable`, `@ts-expect-error`, `prettier-ignore`, …) are exempt too. +```text +BAD: the full paragraph on how the retry budget is derived is copied into the + README, the client, and four call sites. +GOOD: the explanation lives once in the client; each call site carries + // retry budget: see resolveRetryBudget +``` ## Enforcement -This is gated, not merely advised. `verifyx comments` flags low-value comments — long blocks, session narration, -and high comment density — over the current diff (or the whole codebase with `--scope all`). An edit-time -PostToolUse hook (`verifyx comments-hook`) applies the same rules the moment a file is edited. The -`prune-comments` skill strips and consolidates comments across a change on demand. +This is gated, not merely advised. `verifyx comments` flags low-value comments — long blocks, session narration, and high comment density — over the current diff (or the whole codebase with `--scope all`). An edit-time PostToolUse hook (`verifyx comments-hook`) applies the same rules the moment a file is edited. The `prune-comments` skill strips and consolidates comments across a change on demand. diff --git a/.claude/skills/prune-comments/SKILL.md b/.claude/skills/prune-comments/SKILL.md index 7f951e3..706ee1d 100644 --- a/.claude/skills/prune-comments/SKILL.md +++ b/.claude/skills/prune-comments/SKILL.md @@ -21,8 +21,8 @@ Scope options (`verifyx comments` handles the git/diff scoping itself — you do - **Whole repo:** `npx verifyx comments --scope all --pushback` — audit and prune every comment in the codebase, not just the diff. -- **Every comment, not just heuristic hits:** add `--block-all` (with either scope) when the goal is to remove - _all_ comments in scope, keeping only `context:`/JSDoc. +- **Every comment, not just heuristic hits:** add `--block-all` (with either scope) to remove _every_ comment in + scope, not just the heuristic hits. ## 2. Delete or keep @@ -33,10 +33,8 @@ Apply the `comments-only-when-non-obvious` rule to each flagged comment: - **Keep** only a non-obvious _why_: a workaround, an external constraint, or a deliberate non-obvious deviation. Prefer fixing the code over keeping the comment: a better name, a smaller function, or a narrower type often -removes the need for the comment entirely. - -If a kept comment is genuinely durable context the code cannot express, prefix its first line with `context:`. -Do **not** mark low-value comments `context:` to slip them past the gate — `context:` pages a human to approve it. +removes the need for the comment entirely. Do not try to slip a low-value comment past the gate — remove it, or +make the code self-explanatory. ## 3. Apply and re-run diff --git a/templates/rules/comments-only-when-non-obvious.md b/templates/rules/comments-only-when-non-obvious.md index 3413513..d57b9e6 100644 --- a/templates/rules/comments-only-when-non-obvious.md +++ b/templates/rules/comments-only-when-non-obvious.md @@ -1,36 +1,37 @@ +--- +paths: + - '*.{ts,tsx,cts,mts,js,jsx,mjs,cjs,yml,yaml}' + - '**/*.{ts,tsx,cts,mts,js,jsx,mjs,cjs,yml,yaml}' +--- + # Comment only when the code would mislead a competent reader -Write self-documenting code: let well-named identifiers, narrow types, and small functions carry the meaning. -Add a comment only when reading the code alone would make a competent reader mispredict its behaviour, and only -to explain the _why_ the code itself cannot show. +Write self-documenting code: let well-named identifiers, narrow types, and small functions carry the meaning. Add a comment only when reading the code alone would make a competent reader mispredict its behaviour, and only to explain the _why_ the code itself cannot show. ## When a comment earns its place -Add one — kept to a single line where you can — for exactly these: +Add one — kept to a single line where possible — for exactly these: - A non-obvious workaround: a specific bug, race, ordering hazard, or platform quirk the code is shaped around. -- A constraint imposed by an external system not visible in the code: an API contract, a wire format, a rate - limit, a required call order. -- A deliberate deviation from the obvious approach, where the obvious approach is wrong for a reason a reader - would not guess. +- A constraint imposed by an external system not visible in the code: an API contract, a wire format, a rate limit, a required call order. +- A deliberate deviation from the obvious approach, where the obvious approach is wrong for a reason a reader would not guess. -**The keep-it test:** if a competent reader would predict the behaviour correctly without the comment, delete it. +The keep-it test: if a competent reader would predict the behaviour correctly without the comment, delete the comment. ## What never to write -- **Prose blocks on trivial code.** A one-liner, an assignment, or a single call never gets a multi-line comment. -- **Restatement.** A comment that re-says what the next line already says in code is noise; the code is the - source of truth, and a description of it rots and misleads. -- **Session narration.** Never write the editing session's reasoning into the file: `let me…`, `now I'll…`, `as -requested`, `this function does…`, `first,` / `next,`. That belongs in the chat, not the code. -- **LLM tells.** Em-dashes and curly quotes (`—`, `“ ” ‘ ’`) in a comment are a strong sign it was generated - narration rather than a deliberate note. -- **Duplicated explanation.** State a fact once, at its source of truth; a second site gets a one-line pointer, - not a copy. +These are the failure modes this rule exists to stop. They are the current default-model drift, and they poison codebases at scale. + +- **Prose blocks on trivial code.** A one-liner, an assignment, or a single call never gets a multi-line comment. A paragraph above one statement is the dominant failure. +- **Restatement.** A comment that re-says what the next line already says in code is noise, not signal. The code is the source of truth; a description of it rots and must then be re-verified by every reader. +- **Leaked session narration.** Never write the editing session's ephemeral reasoning into code or to disk: `Let me…`, `Now I'll…`, `This is a substantial change…`, `we need to…`, `as requested`. That belongs in the chat, never the file. +- **LLM tells.** Em-dashes and curly quotes (`—`, `“ ” ‘ ’`) in a comment are a strong sign it was generated narration rather than a deliberate note; the gate flags them. +- **Invented shorthand.** Do not coin a new term, abbreviation, or naming scheme inside a comment and leave it undefined; it snowballs and is never cleaned up. Use the codebase's existing vocabulary. +- **Duplicated explanation.** State a fact once, at its source of truth (the README, or the single function that owns it). A second site gets a one-line pointer, not a copy — the same explanation restated across many files is the same bug as restatement, spread out. ## Examples -Restatement / prose block — delete it; the code already says this: +Restatement and a prose block — delete it; the code already says this: ```ts // BAD @@ -45,36 +46,42 @@ const total = quantity * unitPrice A justified comment — keep it; the constraint is external and invisible in the code: ```ts -// GOOD -// context: Stripe rejects sub-cent precision — round to integer cents before sending. +// GOOD: Stripe rejects sub-cent precision; round to integer cents before sending. const amountCents = Math.round(price * 100) ``` -Session narration — strip it: +Leaked session narration — strip it: ```ts // BAD -// Now let me add the retry logic that was requested. This wraps the call in a loop -// and backs off exponentially. -for (let attempt = 0; attempt < maxRetries; attempt++) { … } +// Now let me add the retry logic that was requested. This is the substantial +// part of the change: wrap the call in a loop and back off exponentially. +for (let attempt = 0; attempt < maxRetries; attempt++) { ... } // GOOD -for (let attempt = 0; attempt < maxRetries; attempt++) { … } +for (let attempt = 0; attempt < maxRetries; attempt++) { ... } ``` -## The two escape hatches +Config and infra are not exempt — the same restatement shows up in YAML: -Two things are always allowed and never need pruning: +```yaml +# BAD: the key and its default already say this +# The log level for the service, defaulting to info. +logLevel: info -- **JSDoc** (`/** … */`). -- A comment whose first line starts with **`context:`** — the marker for durable context the code cannot - express. Use it only for a genuinely necessary _why_; it is not a way to keep a restatement. +# GOOD +logLevel: info +``` + +Duplicated explanation — say it once, point to it: -Machine directives (`eslint-disable`, `@ts-expect-error`, `prettier-ignore`, …) are exempt too. +```text +BAD: the full paragraph on how the retry budget is derived is copied into the + README, the client, and four call sites. +GOOD: the explanation lives once in the client; each call site carries + // retry budget: see resolveRetryBudget +``` ## Enforcement -This is gated, not merely advised. `verifyx comments` flags low-value comments — long blocks, session narration, -and high comment density — over the current diff (or the whole codebase with `--scope all`). An edit-time -PostToolUse hook (`verifyx comments-hook`) applies the same rules the moment a file is edited. The -`prune-comments` skill strips and consolidates comments across a change on demand. +This is gated, not merely advised. `verifyx comments` flags low-value comments — long blocks, session narration, and high comment density — over the current diff (or the whole codebase with `--scope all`). An edit-time PostToolUse hook (`verifyx comments-hook`) applies the same rules the moment a file is edited. The `prune-comments` skill strips and consolidates comments across a change on demand. diff --git a/templates/skills/prune-comments/SKILL.md b/templates/skills/prune-comments/SKILL.md index 7f951e3..706ee1d 100644 --- a/templates/skills/prune-comments/SKILL.md +++ b/templates/skills/prune-comments/SKILL.md @@ -21,8 +21,8 @@ Scope options (`verifyx comments` handles the git/diff scoping itself — you do - **Whole repo:** `npx verifyx comments --scope all --pushback` — audit and prune every comment in the codebase, not just the diff. -- **Every comment, not just heuristic hits:** add `--block-all` (with either scope) when the goal is to remove - _all_ comments in scope, keeping only `context:`/JSDoc. +- **Every comment, not just heuristic hits:** add `--block-all` (with either scope) to remove _every_ comment in + scope, not just the heuristic hits. ## 2. Delete or keep @@ -33,10 +33,8 @@ Apply the `comments-only-when-non-obvious` rule to each flagged comment: - **Keep** only a non-obvious _why_: a workaround, an external constraint, or a deliberate non-obvious deviation. Prefer fixing the code over keeping the comment: a better name, a smaller function, or a narrower type often -removes the need for the comment entirely. - -If a kept comment is genuinely durable context the code cannot express, prefix its first line with `context:`. -Do **not** mark low-value comments `context:` to slip them past the gate — `context:` pages a human to approve it. +removes the need for the comment entirely. Do not try to slip a low-value comment past the gate — remove it, or +make the code self-explanatory. ## 3. Apply and re-run From b87f13fb3e82af4b735426fee1a67a3adb5505ee Mon Sep 17 00:00:00 2001 From: "Rob Moore (MakerX)" Date: Sat, 11 Jul 2026 15:06:19 +0800 Subject: [PATCH 09/11] =?UTF-8?q?fix:=20address=20Copilot=20review=20?= =?UTF-8?q?=E2=80=94=20scope-all=20ignores,=20block-comment=20scope,=20hoo?= =?UTF-8?q?k=20path,=20init=20message?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - --scope all now honours verify.config.json#comments.ignore, not just --ignore. - Diff scope counts a comment as touched when any of its lines changed, not only its opening line (block comments edited in the body now reach narration/block-all). - The hook normalises the payload path (absolute + Windows separators) and matches ignore globs against both absolute and cwd-relative forms. - init's 'wrote to nearest .claude' note only prints when agent files were actually written. - Split resolveSelections to keep registerInit.ts above the maintainability threshold after the rebase. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/checks/comment-collect.ts | 11 +++++++-- src/checks/comments.ts | 2 +- src/commands/registerCommentsHook.ts | 12 +++++++++- src/commands/registerInit.ts | 34 ++++++++++++++++------------ 4 files changed, 40 insertions(+), 19 deletions(-) diff --git a/src/checks/comment-collect.ts b/src/checks/comment-collect.ts index e4cf8ed..92d77d9 100644 --- a/src/checks/comment-collect.ts +++ b/src/checks/comment-collect.ts @@ -64,8 +64,15 @@ export function gatherDiffComments(ignoreGlobs: readonly string[], maxLines: num for (const c of scanComments(file, content)) { if (isDiffExempt(c.text)) continue const span = commentSpan(c.text) - for (let l = c.line; l < c.line + span; l++) if (changed.has(l)) commentLines++ - if (changed.has(c.line)) comments.push({ file, line: c.line, text: toSingleLine(c.text) }) + // context: a comment is in scope when any of its lines changed, not only its opening line, so an edit to a + // context: block comment's body still surfaces it to the narration and block-all gates. + let touched = false + for (let l = c.line; l < c.line + span; l++) { + if (!changed.has(l)) continue + commentLines++ + touched = true + } + if (touched) comments.push({ file, line: c.line, text: toSingleLine(c.text) }) } let addedNonBlank = 0 for (const l of changed) if (!isBlank(fileLines[l - 1])) addedNonBlank++ diff --git a/src/checks/comments.ts b/src/checks/comments.ts index 86bf9f3..2b56bb4 100644 --- a/src/checks/comments.ts +++ b/src/checks/comments.ts @@ -45,7 +45,7 @@ const whereText = (scope: CommentScope): string => (scope === 'all' ? 'in the co function gather(opts: CommentsOptions, scope: CommentScope, ignoreGlobs: readonly string[], maxLines: number): CommentCandidates { if (scope === 'all') { - const files = findSourceFiles(resolvePattern(opts.pattern ?? DEFAULT_PATTERN), [...DEFAULT_IGNORE, ...(opts.ignore ?? [])]) + const files = findSourceFiles(resolvePattern(opts.pattern ?? DEFAULT_PATTERN), [...DEFAULT_IGNORE, ...ignoreGlobs]) return gatherAllComments(files, maxLines) } return gatherDiffComments(ignoreGlobs, maxLines) diff --git a/src/commands/registerCommentsHook.ts b/src/commands/registerCommentsHook.ts index 1e23990..c7b4b2f 100644 --- a/src/commands/registerCommentsHook.ts +++ b/src/commands/registerCommentsHook.ts @@ -1,3 +1,5 @@ +import path from 'node:path' + import type { Command } from 'commander' import { minimatch } from 'minimatch' @@ -15,6 +17,14 @@ async function readStdin(): Promise { return Buffer.concat(chunks).toString('utf-8') } +// context: Claude's payload path is absolute and may use Windows separators; match ignore globs (e.g. +// **/*.generated.ts) against both the forward-slashed absolute path and a cwd-relative form, as the check does. +function isIgnored(file: string, globs: readonly string[]): boolean { + const abs = file.replaceAll('\\', '/') + const rel = path.relative(process.cwd(), file).replaceAll('\\', '/') + return globs.some((glob) => minimatch(abs, glob) || minimatch(rel, glob)) +} + function resolveOptions(): { options: HookOptions; ignore: string[] } { const cfg = loadVerifyConfig().comments ?? {} const density = cfg.density === false ? 0 : (cfg.density ?? DEFAULT_DENSITY) @@ -43,7 +53,7 @@ export function registerCommentsHook(program: Command): void { const target = parsePostToolUsePayload(await readStdin()) if (!target) return const { options, ignore } = resolveOptions() - if (ignore.some((glob) => minimatch(target.file, glob))) return + if (isIgnored(target.file, ignore)) return const findings = analyzeAddedComments(target, options) if (!hasFindings(findings)) return diff --git a/src/commands/registerInit.ts b/src/commands/registerInit.ts index 816d5fb..d8f9d1d 100644 --- a/src/commands/registerInit.ts +++ b/src/commands/registerInit.ts @@ -63,22 +63,21 @@ async function askCommentOptions(): Promise<{ commentScope: CommentScope; commen return { commentScope, commentBlockAll } } -async function resolveSelections(opts: InitCliOptions): Promise { - const nonInteractive = !!opts.yes || !process.stdin.isTTY - if (nonInteractive) { - const targets: AgentTarget[] = [] - if (opts.claude !== false) targets.push('claude') - if (opts.agents) targets.push('agents') - return { - checks: opts.select.length > 0 ? opts.select : recommendedChecks().map((c) => c.name), - targets, - defaultsOnly: !!opts.defaultsOnly, - commentHook: opts.commentHook !== false, - commentScope: opts.commentScope === 'all' ? 'all' : 'diff', - commentBlockAll: !!opts.commentBlockAll, - } +function nonInteractiveSelections(opts: InitCliOptions): Selections { + const targets: AgentTarget[] = [] + if (opts.claude !== false) targets.push('claude') + if (opts.agents) targets.push('agents') + return { + checks: opts.select.length > 0 ? opts.select : recommendedChecks().map((c) => c.name), + targets, + defaultsOnly: !!opts.defaultsOnly, + commentHook: opts.commentHook !== false, + commentScope: opts.commentScope === 'all' ? 'all' : 'diff', + commentBlockAll: !!opts.commentBlockAll, } +} +async function interactiveSelections(): Promise { // context: an up-front choice — run everything with defaults (verifyx all, no verify:* scripts), or hand-pick checks. const mode = await ask('select', 'How should verify run?', [ { name: 'defaults', message: 'Run all built-in checks (verifyx all) with default options', enabled: true }, @@ -113,6 +112,11 @@ async function resolveSelections(opts: InitCliOptions): Promise { return { checks, targets, defaultsOnly, commentHook, commentScope, commentBlockAll } } +async function resolveSelections(opts: InitCliOptions): Promise { + const nonInteractive = !!opts.yes || !process.stdin.isTTY + return nonInteractive ? nonInteractiveSelections(opts) : interactiveSelections() +} + function report(result: InitResult, defaultsOnly: boolean): void { if (defaultsOnly) { console.log(color.dim('\nDefaults-only: no verify:* scripts written — the `verify` script runs `verifyx all` (every built-in).')) @@ -122,7 +126,7 @@ function report(result: InitResult, defaultsOnly: boolean): void { if (file.action === 'unchanged') continue console.log(` ${ACTION_MARK[file.action]} ${file.path} (${file.action})`) } - if (result.rootDir !== process.cwd()) { + if (result.rootDir !== process.cwd() && result.agentFiles.length > 0) { console.log( color.yellow( `\nAgent files were written to ${result.rootDir} (nearest existing .claude). Claude Code only loads them when launched from there — pass --claude-dir to target a different directory.`, From 7ede28d1023ae606d6cc5a40c69e3d01f482cdd9 Mon Sep 17 00:00:00 2001 From: "Rob Moore (MakerX)" Date: Sat, 11 Jul 2026 15:37:15 +0800 Subject: [PATCH 10/11] feat: opt out of the context: override (--no-context-override) + strict no-comments init preset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds comments.contextOverride (default true; CLI --no-context-override). When off, context: comments are judged like any other — a stricter stance and a way to drive a context: cleanup; JSDoc and machine directives stay exempt. All gate output goes context-free in that mode: the reports drop their 'prefix with context:' lines and --pushback swaps to a strict variant that offers no escape. verifyx init gains a 3-way strictness choice (heuristics / block-all / strict) and --comment-strict: the strict preset bakes --block-all --no-context-override into verify:comments — a no-comments-at-all, JSDoc-only policy. Renames the internal allowContext to contextOverride throughout. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 26 +++++++++---- src/checks/comment-collect.ts | 16 ++++---- src/checks/comment-common.ts | 13 +++---- src/checks/comment-heuristics.test.ts | 7 ++++ src/checks/comment-heuristics.ts | 7 ++-- src/checks/comments.test.ts | 2 - src/checks/comments.ts | 53 +++++++++++++++++---------- src/checks/registry.ts | 4 +- src/commands/registerChecks.ts | 5 ++- src/commands/registerCommentsHook.ts | 3 +- src/commands/registerInit.ts | 38 ++++++++++--------- src/comments.ts | 26 ++++++++----- src/hook/analyze.test.ts | 20 ++++++++-- src/hook/analyze.ts | 23 ++++++++---- src/report.ts | 45 +++++++++++++++++------ src/scaffold/init.test.ts | 5 +++ src/scaffold/init.ts | 18 ++++++--- src/shared/config.ts | 2 + 18 files changed, 207 insertions(+), 106 deletions(-) diff --git a/README.md b/README.md index 3b55dc8..7a2afa9 100644 --- a/README.md +++ b/README.md @@ -178,6 +178,7 @@ The check has two orthogonal axes. **Scope** decides which comments are candidat - `--pushback`: reframe the failure message as back-pressure aimed at an AI agent (see below). - `--warn`: report the long-block violations without failing the run. - `--no-narration`: turn off the session-narration gate (on by default; see below). +- `--no-context-override`: stop treating `context:`-prefixed comments as exempt — a stricter stance, or to drive a cleanup of existing `context:` use. - `--comment-density `: the comment share (0–1) that fails a file (default `0.3`; `0` disables). - `--min-added-lines `: skip the density gate when fewer than `n` lines are in scope (default 10). - `--block-all`: fail _every_ non-exempt comment in scope — the strictest gate; supersedes the heuristics. @@ -192,19 +193,21 @@ Three heuristics run by default within the scope: The default `diff` scope means the heuristics judge only what a change introduces, never legacy comments — so you can adopt the check on an existing repo without a cleanup first. Use `--scope all` for a full-codebase audit. All gates honour the same escape hatches (JSDoc, `context:`, machine directives); tune or disable them per repo via [verify config](#configuration). -**`--pushback` is the clever bit.** An AI agent that hits a failing check will often take the path of least resistance and just delete or weaken the check to make the run pass. So rather than a dry error, the pushback message tells the agent that the _only_ sanctioned way to keep the comment is to prefix it with `context:`, and that doing so **pages a human to approve it**. Confronted with a real person's time on the line, the agent tends to reconsider and remove the low-value comment instead of gaming the gate. It is a small piece of prompt-engineering baked into a lint failure, and in practice it stops agents silencing the check far more reliably than a plain error does. +**`--pushback` gives stronger back-pressure.** An AI agent that hits a failing check will often take the path of least resistance and just delete or weaken the check to make the run pass. So rather than a dry error, the pushback message tells the agent that the _only_ sanctioned way to keep the comment is to prefix it with `context:`, and that doing so **pages a human to approve it**. Confronted with a real person's time on the line, the agent tends to reconsider more often and removes the low-value comment instead of gaming the gate. It is a small piece of prompt-engineering baked into a lint failure, and in practice it we found it stops agents silencing the check more reliably than a plain error does. If you want to go further and block comments outright, add `--block-all`: every non-exempt comment in scope fails, not just the heuristic hits. Pair it with the scope you want — `--scope diff --block-all` (the `--block-new-comments` alias) fails any comment on a changed line, while `--scope all --block-all` is a zero-comment policy across the codebase. Machine directives (`eslint-disable`, `@ts-expect-error`, and friends) stay exempt, as does anything marked `context:`. Under the default `diff` scope, the "changed lines" are resolved per environment so the gate works the same locally and in CI. Locally it diffs the working tree against `HEAD` (your uncommitted changes). In CI (`CI` set) a clean checkout has nothing uncommitted, so it diffs against the **merge base with the PR base branch** instead, read from `GITHUB_BASE_REF` (GitHub Actions). Set `VERIFY_DIFF_BASE` to a ref to override the base explicitly; if no base can be resolved it falls back to `HEAD`. For CI, make sure the base branch is fetched (`actions/checkout` with `fetch-depth: 0`), or the merge base won't be found and the gate silently passes. -Two escape hatches keep genuinely useful comments alive. **JSDoc** (`/** … */`) is always allowed, and prefixing a comment's first line with `context:` marks it as durable context the code itself can't express: +Two escape hatches keep genuinely useful comments alive. **JSDoc** (`/** … */`) is always allowed, and prefixing a comment's first line with `context:` marks it as durable context the code itself can't express — the kind of constraint a reader would otherwise "fix" and break, where no rename or refactor would carry the _why_: ```ts -// context: the upstream API returns seconds, not milliseconds, so do not "fix" this -const timeoutMs = timeout * 1000 +// context: keep this sequential; the gateway 429s on concurrent calls, so Promise.all here breaks it. +for (const id of ids) await fetchOne(id) ``` +**Opting out of `context:`.** For a stricter stance — or to drive a cleanup of existing `context:` comments — pass `--no-context-override` (or set `comments.contextOverride: false` in your [verify config](#configuration)). `context:` comments are then judged like any other; JSDoc and machine directives stay exempt. Because `context:` no longer offers an escape, this also removes the one sanctioned way to keep a flagged comment (and the `--pushback` message drops its "prefix with `context:`" framing accordingly), so use it when you genuinely want the gate at its strictest. Combined with `--block-all`, this is a **no-comments-at-all, JSDoc-only** policy — `verifyx init` offers it directly as its strictest comment option (or `--comment-strict`), baking `--block-all --no-context-override` into the scaffolded `verify:comments` script. + #### Edit-time hook (Claude PostToolUse) The `comments` check runs at verify/CI time — the end of a task. To close the loop tighter, `verifyx init` can also register an **edit-time gate** as a Claude Code [PostToolUse hook](https://code.claude.com/docs/en/hooks), so the same rules fire the moment an agent writes a file: @@ -213,7 +216,12 @@ The `comments` check runs at verify/CI time — the end of a task. To close the // .claude/settings.json (added by `verifyx init`; opt out with --no-comment-hook) { "hooks": { - "PostToolUse": [{ "matcher": "Edit|Write|MultiEdit", "hooks": [{ "type": "command", "command": "npx verifyx comments-hook" }] }], + "PostToolUse": [ + { + "matcher": "Edit|Write|MultiEdit", + "hooks": [{ "type": "command", "command": "npx verifyx comments-hook" }], + }, + ], }, } ``` @@ -224,7 +232,7 @@ The `comments` check runs at verify/CI time — the end of a task. To close the #### `prune-comments` skill + comment rule -`verifyx init` also drops a **`prune-comments`** skill and a **`comments-only-when-non-obvious`** rule (`.claude/rules` + `.agent-skills/rules`) — the remediation companion to the gate, with explicit keep/delete criteria and worked examples. Ask an agent to "prune comments" and it runs `verifyx comments` over the diff, then applies the rule to clean up low-value comments (with `context:` as the sanctioned escape hatch). To sweep the **whole codebase** rather than just a change, it runs `verifyx comments --scope all`. +`verifyx init` also drops a **`prune-comments`** skill and a **`comments-only-when-non-obvious`** rule (`.claude/rules` + `.agent-skills/rules`) — the remediation companion to the gate, with explicit keep/delete criteria and worked examples. Ask an agent to "prune comments" and it runs `verifyx comments` over the diff, then applies the rule to clean up low-value comments. To sweep the **whole codebase** rather than just the current diff, it runs `verifyx comments --scope all`. ### `hardcoded-colors` @@ -314,6 +322,7 @@ The **native** checks that take persistent settings read them from a `verify.con "narration": true, "density": 0.3, "minAddedLines": 10, + "contextOverride": true, }, "hardcodedColors": { "root": "src", "ignore": ["**/tokens.ts"] }, "forbiddenStrings": [{ "file": "app.json", "paths": ["env.LOG_LEVEL"], "disallowed": "debug" }], @@ -355,7 +364,10 @@ Every export is available from the package root: import { analyzeComplexity, getCheck, runAll } from '@makerx/verify' // Run the maintainability analysis directly. -const { failing } = analyzeComplexity({ pattern: 'src/**/*.ts', threshold: 50 }) +const { failing } = analyzeComplexity({ + pattern: 'src/**/*.ts', + threshold: 50, +}) // Run any single check by name, including the ones that shell out to an external tool. const lint = await getCheck('lint')?.runDefault() diff --git a/src/checks/comment-collect.ts b/src/checks/comment-collect.ts index 92d77d9..c58336d 100644 --- a/src/checks/comment-collect.ts +++ b/src/checks/comment-collect.ts @@ -25,17 +25,17 @@ const blockIntersects = (block: CommentBlockViolation, changed: ReadonlySet comments.sort((a, b) => a.file.localeCompare(b.file) || a.line - b.line) -// context: scope `all` judges every comment in the matched files — the whole-codebase audit / prune surface. -export function gatherAllComments(files: readonly string[], maxLines: number): CommentCandidates { +// scope `all` judges every comment in the matched files: the whole-codebase audit / prune surface. +export function gatherAllComments(files: readonly string[], maxLines: number, contextOverride = true): CommentCandidates { const blocks: CommentBlockViolation[] = [] const comments: NewComment[] = [] const perFile = new Map() for (const file of files) { const content = fs.readFileSync(file, 'utf-8') - blocks.push(...findLongCommentBlocksInContent(file, content, maxLines)) + blocks.push(...findLongCommentBlocksInContent(file, content, maxLines, contextOverride)) let commentLines = 0 for (const c of scanComments(file, content)) { - if (isDiffExempt(c.text)) continue + if (isDiffExempt(c.text, contextOverride)) continue commentLines += commentSpan(c.text) comments.push({ file, line: c.line, text: toSingleLine(c.text) }) } @@ -45,8 +45,8 @@ export function gatherAllComments(files: readonly string[], maxLines: number): C return { blocks, comments: sortComments(comments), perFile } } -// context: scope `diff` judges only comments touching changed lines (vs the diff base) — one pass feeds every gate. -export function gatherDiffComments(ignoreGlobs: readonly string[], maxLines: number): CommentCandidates { +// scope `diff` judges only comments touching changed lines (vs the diff base); one pass feeds every gate. +export function gatherDiffComments(ignoreGlobs: readonly string[], maxLines: number, contextOverride = true): CommentCandidates { const diff = gitDiffAgainstBase() const added = parseDiffAddedLines(diff) const removedByFile = parseDiffRemovedLines(diff) @@ -57,12 +57,12 @@ export function gatherDiffComments(ignoreGlobs: readonly string[], maxLines: num if (!shouldScan(file, ignoreGlobs)) continue const content = fs.readFileSync(file, 'utf-8') const fileLines = content.split('\n') - for (const block of findLongCommentBlocksInContent(file, content, maxLines)) { + for (const block of findLongCommentBlocksInContent(file, content, maxLines, contextOverride)) { if (blockIntersects(block, changed)) blocks.push(block) } let commentLines = 0 for (const c of scanComments(file, content)) { - if (isDiffExempt(c.text)) continue + if (isDiffExempt(c.text, contextOverride)) continue const span = commentSpan(c.text) // context: a comment is in scope when any of its lines changed, not only its opening line, so an edit to a // context: block comment's body still surfaces it to the narration and block-all gates. diff --git a/src/checks/comment-common.ts b/src/checks/comment-common.ts index ef4274a..c0f46eb 100644 --- a/src/checks/comment-common.ts +++ b/src/checks/comment-common.ts @@ -4,8 +4,7 @@ import { minimatch } from 'minimatch' const SCANNED_EXTENSIONS = ['.ts', '.tsx', '.cts', '.mts', '.js', '.jsx', '.mjs', '.cjs', '.yml', '.yaml'] -// context: machine directives steer tooling, not humans, so a changed line carrying one is never a "comment" -// worth removing; `context:` is this project's durable-context escape hatch and is likewise exempt. +// machine directives steer tooling, not humans, so a changed line carrying one is never a "comment" const MACHINE_DIRECTIVES = [ 'oxlint-disable', 'oxlint-enable', @@ -23,9 +22,10 @@ const MACHINE_DIRECTIVES = [ export type NewComment = { file: string; line: number; text: string } -export function isCommentExempt(text: string): boolean { +export function isCommentExempt(text: string, contextOverride = true): boolean { const lower = text.toLowerCase() if (MACHINE_DIRECTIVES.some((d) => lower.includes(d))) return true + if (!contextOverride) return false const stripped = text.replace(/^\s*(?:\/\/+|\/\*+|\*|#)\s*/, '') return stripped.toLowerCase().startsWith('context:') } @@ -34,14 +34,13 @@ export function isScannedExtension(file: string): boolean { return SCANNED_EXTENSIONS.some((ext) => file.endsWith(ext)) } -/** JSDoc (`/** … *\/`) is an always-allowed escape hatch, so the diff-scoped gates skip it like `context:`. */ +/** JSDoc (`/** … *\/`) is an always-allowed escape hatch. */ function isJsDoc(text: string): boolean { return text.trimStart().startsWith('/**') } -/** Exempt from the diff-scoped gates: machine directives, `context:` blocks, and JSDoc. */ -export function isDiffExempt(text: string): boolean { - return isCommentExempt(text) || isJsDoc(text) +export function isDiffExempt(text: string, contextOverride = true): boolean { + return isCommentExempt(text, contextOverride) || isJsDoc(text) } export function shouldScan(file: string, ignoreGlobs: readonly string[]): boolean { diff --git a/src/checks/comment-heuristics.test.ts b/src/checks/comment-heuristics.test.ts index d8b4afd..cb6496f 100644 --- a/src/checks/comment-heuristics.test.ts +++ b/src/checks/comment-heuristics.test.ts @@ -34,6 +34,13 @@ describe('findNarrationComments', () => { const tells = [comment('// wraps the call — then retries'), comment('// uses the “fast” path')] expect(findNarrationComments(tells)).toHaveLength(2) }) + + it('with allowContext false, stops exempting context: comments (machine directives still exempt)', () => { + const comments = [comment('// context: let me keep this durable note'), comment('// eslint-disable-next-line -- let me disable')] + const flagged = findNarrationComments(comments, false) + expect(flagged).toHaveLength(1) + expect(flagged[0]?.text).toContain('context:') + }) }) describe('findCommentDensity', () => { diff --git a/src/checks/comment-heuristics.ts b/src/checks/comment-heuristics.ts index d35aa79..7d77caa 100644 --- a/src/checks/comment-heuristics.ts +++ b/src/checks/comment-heuristics.ts @@ -15,8 +15,7 @@ const NARRATION_PATTERNS: RegExp[] = [ /\b(?:step \d|todo:|fixme:)/i, ] -// context: em-dash and curly quotes are high-precision LLM tells — models emit them freely, hand-typed code -// comments rarely do. Borrowed from claude-comment-gate's H2 punctuation check. +// context: em-dash and curly quotes are high-precision LLM tells — models emit them freely. const LLM_PUNCT_TELL = /[—‘’“”]/ export type DensityViolation = { @@ -30,9 +29,9 @@ export type DensityViolation = { } /** From comments in scope, return those whose text reads as session narration or carries an LLM punctuation tell. */ -export function findNarrationComments(comments: readonly NewComment[]): NewComment[] { +export function findNarrationComments(comments: readonly NewComment[], contextOverride = true): NewComment[] { return comments.filter( - (c) => !isCommentExempt(c.text) && (NARRATION_PATTERNS.some((re) => re.test(c.text)) || LLM_PUNCT_TELL.test(c.text)), + (c) => !isCommentExempt(c.text, contextOverride) && (NARRATION_PATTERNS.some((re) => re.test(c.text)) || LLM_PUNCT_TELL.test(c.text)), ) } diff --git a/src/checks/comments.test.ts b/src/checks/comments.test.ts index f33986d..70bd041 100644 --- a/src/checks/comments.test.ts +++ b/src/checks/comments.test.ts @@ -7,8 +7,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { gitDiffAgainstBase } from '../shared/git.ts' import { runComments } from './comments.ts' -// context: the diff-scoped gates (narration/density/block-new) read git; mocking the diff keeps these tests -// hermetic and lets each case craft the exact changed lines it needs. vi.mock('../shared/git.ts', () => ({ gitDiffAgainstBase: vi.fn(() => '') })) const mockDiff = (diff: string): void => { vi.mocked(gitDiffAgainstBase).mockReturnValue(diff) diff --git a/src/checks/comments.ts b/src/checks/comments.ts index 2b56bb4..c7d39a8 100644 --- a/src/checks/comments.ts +++ b/src/checks/comments.ts @@ -34,6 +34,8 @@ export type CommentsOptions = { density?: number | false /** Minimum lines in scope before density applies. Default 10. */ minAddedLines?: number + /** Treat `context:`-prefixed comments as exempt. Default true; set false for a stricter gate / cleanup. */ + contextOverride?: boolean } function resolveDensity(opt: number | false | undefined, cfg: number | false | undefined): number { @@ -43,12 +45,18 @@ function resolveDensity(opt: number | false | undefined, cfg: number | false | u const whereText = (scope: CommentScope): string => (scope === 'all' ? 'in the codebase' : 'on changed lines') -function gather(opts: CommentsOptions, scope: CommentScope, ignoreGlobs: readonly string[], maxLines: number): CommentCandidates { +function gather( + opts: CommentsOptions, + scope: CommentScope, + ignoreGlobs: readonly string[], + maxLines: number, + contextOverride: boolean, +): CommentCandidates { if (scope === 'all') { const files = findSourceFiles(resolvePattern(opts.pattern ?? DEFAULT_PATTERN), [...DEFAULT_IGNORE, ...ignoreGlobs]) - return gatherAllComments(files, maxLines) + return gatherAllComments(files, maxLines, contextOverride) } - return gatherDiffComments(ignoreGlobs, maxLines) + return gatherDiffComments(ignoreGlobs, maxLines, contextOverride) } /** @@ -66,28 +74,33 @@ export function runComments(opts: CommentsOptions = {}): CheckResult { const narrationOn = !blockAll && (opts.narration ?? cfg.narration ?? true) const densityThreshold = blockAll ? 0 : resolveDensity(opts.density, cfg.density) const minAddedLines = opts.minAddedLines ?? cfg.minAddedLines ?? DEFAULT_MIN_ADDED_LINES + const contextOverride = opts.contextOverride ?? cfg.contextOverride ?? true const pushback = !!opts.pushback - const candidates = gather(opts, scope, ignoreGlobs, maxLines) + const candidates = gather(opts, scope, ignoreGlobs, maxLines, contextOverride) - // context: --block-all is the strictest gate (every comment fails), so it stands alone — no heuristics run. if (blockAll) { if (candidates.comments.length === 0) { console.log(color.green(`No comments ${whereText(scope)}.`)) return { name: 'comments', ok: true } } - printBlockAllReport(candidates.comments, { scope, pushback }) + printBlockAllReport(candidates.comments, { scope, pushback, contextOverride }) return { name: 'comments', ok: false } } let ok = true - ok = reportBlocks(candidates, maxLines, { pushback, warn: !!opts.warn }) && ok - if (narrationOn) ok = reportNarration(candidates, scope, pushback) && ok - if (densityThreshold > 0) ok = reportDensity(candidates, scope, pushback, densityThreshold, minAddedLines) && ok + ok = reportBlocks(candidates, maxLines, { pushback, warn: !!opts.warn, contextOverride }) && ok + if (narrationOn) ok = reportNarration(candidates, scope, pushback, contextOverride) && ok + if (densityThreshold > 0) + ok = reportDensity(candidates, { scope, pushback, contextOverride, threshold: densityThreshold, minAddedLines }) && ok return { name: 'comments', ok } } -function reportBlocks(candidates: CommentCandidates, maxLines: number, opts: { pushback: boolean; warn: boolean }): boolean { +function reportBlocks( + candidates: CommentCandidates, + maxLines: number, + opts: { pushback: boolean; warn: boolean; contextOverride: boolean }, +): boolean { if (candidates.blocks.length === 0) { console.log(color.green(`No comment block over ${maxLines} line(s)`)) return true @@ -96,28 +109,30 @@ function reportBlocks(candidates: CommentCandidates, maxLines: number, opts: { p return opts.warn } -function reportNarration(candidates: CommentCandidates, scope: CommentScope, pushback: boolean): boolean { - const narration = findNarrationComments(candidates.comments) +function reportNarration(candidates: CommentCandidates, scope: CommentScope, pushback: boolean, contextOverride: boolean): boolean { + const narration = findNarrationComments(candidates.comments, contextOverride) if (narration.length === 0) { console.log(color.green(`No narration comments ${whereText(scope)}.`)) return true } - printNarrationReport(narration, { scope, pushback }) + printNarrationReport(narration, { scope, pushback, contextOverride }) return false } function reportDensity( candidates: CommentCandidates, - scope: CommentScope, - pushback: boolean, - threshold: number, - minAddedLines: number, + opts: { scope: CommentScope; pushback: boolean; contextOverride: boolean; threshold: number; minAddedLines: number }, ): boolean { - const dense = findCommentDensity(candidates.perFile, { threshold, minAddedLines }) + const dense = findCommentDensity(candidates.perFile, { threshold: opts.threshold, minAddedLines: opts.minAddedLines }) if (dense.length === 0) { console.log(color.green('Comment density within threshold.')) return true } - printCommentDensityReport(dense, { threshold, scope, pushback }) + printCommentDensityReport(dense, { + threshold: opts.threshold, + scope: opts.scope, + pushback: opts.pushback, + contextOverride: opts.contextOverride, + }) return false } diff --git a/src/checks/registry.ts b/src/checks/registry.ts index bd6efa1..63b9386 100644 --- a/src/checks/registry.ts +++ b/src/checks/registry.ts @@ -20,12 +20,12 @@ function nativeCheck(name: string, description: string, recommended: boolean, ru } } -// context: checks are named for their function, never the tool behind them (see each check's bin/devDeps). +// Checks are named for their function, never the tool behind them (see each check's bin/devDeps). export const CHECKS: Check[] = [ nativeCheck('complexity', 'Maintainability-index gate (cyclomatic + Halstead + SLOC)', true, () => runComplexity()), nativeCheck( 'comments', - 'Flag low-value comments — long blocks + narration/density (JSDoc / context: exempt); --scope diff|all, --block-all for a zero-comment gate', + 'Flag low-value comments — long blocks + narration/density; --scope diff|all, --block-all for a zero-comment gate', true, () => runComments({ pushback: true }), 'verifyx comments --pushback', diff --git a/src/commands/registerChecks.ts b/src/commands/registerChecks.ts index 3cd76e5..496e820 100644 --- a/src/commands/registerChecks.ts +++ b/src/commands/registerChecks.ts @@ -39,6 +39,7 @@ export function registerChecks(program: Command): void { .option('--block-all', 'fail every non-exempt comment in scope, not just heuristic hits') .option('--block-new-comments', 'alias for --scope diff --block-all') .option('--no-narration', 'do not flag session-narration comments') + .option('--no-context-override', 'disable the `context:` override, so those comments are no longer exempt (stricter gate / cleanup)') .option('--comment-density ', 'comment-density ratio (0–1) that fails a file; 0 disables', Number) .option('--min-added-lines ', 'minimum lines in scope before density applies', Number) .option('--ignore ', 'ignore glob (repeatable)', collect, []) @@ -53,6 +54,7 @@ export function registerChecks(program: Command): void { blockAll?: boolean blockNewComments?: boolean narration?: boolean + contextOverride?: boolean commentDensity?: number minAddedLines?: number ignore: string[] @@ -67,9 +69,8 @@ export function registerChecks(program: Command): void { scope: opts.scope === 'all' ? 'all' : opts.scope === 'diff' ? 'diff' : undefined, blockAll: opts.blockAll, blockNewComments: opts.blockNewComments, - // context: commander defaults --no-narration's value to true; forward it only when explicitly - // disabled so verify.config.json's `narration` stays authoritative when the flag is absent. narration: opts.narration === false ? false : undefined, + contextOverride: opts.contextOverride === false ? false : undefined, density: opts.commentDensity, minAddedLines: opts.minAddedLines, ignore: opts.ignore, diff --git a/src/commands/registerCommentsHook.ts b/src/commands/registerCommentsHook.ts index c7b4b2f..44825de 100644 --- a/src/commands/registerCommentsHook.ts +++ b/src/commands/registerCommentsHook.ts @@ -35,6 +35,7 @@ function resolveOptions(): { options: HookOptions; ignore: string[] } { density, minAddedLines: cfg.minAddedLines ?? DEFAULT_MIN_ADDED_LINES, blockAll: cfg.blockAll ?? false, + contextOverride: cfg.contextOverride ?? true, }, ignore: cfg.ignore ?? [], } @@ -58,7 +59,7 @@ export function registerCommentsHook(program: Command): void { const findings = analyzeAddedComments(target, options) if (!hasFindings(findings)) return - process.stderr.write(`${formatHookFeedback(findings)}\n`) + process.stderr.write(`${formatHookFeedback(findings, options.contextOverride)}\n`) process.exitCode = 2 }) } diff --git a/src/commands/registerInit.ts b/src/commands/registerInit.ts index d8f9d1d..ad56203 100644 --- a/src/commands/registerInit.ts +++ b/src/commands/registerInit.ts @@ -38,29 +38,31 @@ type InitCliOptions = { claudeDir?: string commentScope?: string commentBlockAll?: boolean + commentStrict?: boolean } type CommentScope = 'diff' | 'all' +type CommentChoices = { commentScope: CommentScope; commentBlockAll: boolean; commentContextOverride: boolean } type Selections = { checks: string[] targets: AgentTarget[] defaultsOnly: boolean commentHook: boolean - commentScope: CommentScope - commentBlockAll: boolean -} +} & CommentChoices + +const NO_COMMENT_CHOICES: CommentChoices = { commentScope: 'diff', commentBlockAll: false, commentContextOverride: true } -async function askCommentOptions(): Promise<{ commentScope: CommentScope; commentBlockAll: boolean }> { +async function askCommentOptions(): Promise { const commentScope = (await ask('select', 'Comments — which comments should the check gate?', [ { name: 'diff', message: 'Changed lines only (gate new code, skip legacy)', enabled: true }, { name: 'all', message: 'The whole codebase' }, ])) as CommentScope - const commentBlockAll = - (await ask('select', 'Comments — how strict?', [ - { name: 'heuristics', message: 'Heuristics: long blocks, narration, density', enabled: true }, - { name: 'blockAll', message: 'Block every comment in scope (context:/JSDoc still allowed)' }, - ])) === 'blockAll' - return { commentScope, commentBlockAll } + const strictness = await ask('select', 'Comments — how strict?', [ + { name: 'heuristics', message: 'Heuristics: long blocks, narration, density', enabled: true }, + { name: 'blockAll', message: 'Block every comment in scope (JSDoc + context: still allowed)' }, + { name: 'strict', message: 'Strict: no comments at all — only JSDoc allowed' }, + ]) + return { commentScope, commentBlockAll: strictness !== 'heuristics', commentContextOverride: strictness !== 'strict' } } function nonInteractiveSelections(opts: InitCliOptions): Selections { @@ -73,12 +75,13 @@ function nonInteractiveSelections(opts: InitCliOptions): Selections { defaultsOnly: !!opts.defaultsOnly, commentHook: opts.commentHook !== false, commentScope: opts.commentScope === 'all' ? 'all' : 'diff', - commentBlockAll: !!opts.commentBlockAll, + // --comment-strict is the "no comments, JSDoc only" preset: block every comment and drop the context: override. + commentBlockAll: !!opts.commentBlockAll || !!opts.commentStrict, + commentContextOverride: !opts.commentStrict, } } async function interactiveSelections(): Promise { - // context: an up-front choice — run everything with defaults (verifyx all, no verify:* scripts), or hand-pick checks. const mode = await ask('select', 'How should verify run?', [ { name: 'defaults', message: 'Run all built-in checks (verifyx all) with default options', enabled: true }, { name: 'pick', message: 'Pick specific checks to wire up as verify:* scripts' }, @@ -106,10 +109,8 @@ async function interactiveSelections(): Promise { ])) === 'yes' : false - const { commentScope, commentBlockAll } = checks.includes('comments') - ? await askCommentOptions() - : { commentScope: 'diff' as CommentScope, commentBlockAll: false } - return { checks, targets, defaultsOnly, commentHook, commentScope, commentBlockAll } + const comments = checks.includes('comments') ? await askCommentOptions() : NO_COMMENT_CHOICES + return { checks, targets, defaultsOnly, commentHook, ...comments } } async function resolveSelections(opts: InitCliOptions): Promise { @@ -169,9 +170,11 @@ export function registerInit(program: Command): void { .option('--claude-dir ', 'directory to write .claude/.agent-skills into (default: nearest existing .claude, else cwd)') .option('--comment-scope ', 'comments check scope baked into verify:comments: diff (default) or all') .option('--comment-block-all', 'bake --block-all into verify:comments (fail every comment in scope)') + .option('--comment-strict', 'strictest: no comments, only JSDoc (bakes --block-all --no-context-override)') .action(async (opts: InitCliOptions) => { const cwd = process.cwd() - const { checks, targets, defaultsOnly, commentHook, commentScope, commentBlockAll } = await resolveSelections(opts) + const { checks, targets, defaultsOnly, commentHook, commentScope, commentBlockAll, commentContextOverride } = + await resolveSelections(opts) const result = applyInit({ cwd, @@ -182,6 +185,7 @@ export function registerInit(program: Command): void { claudeDir: opts.claudeDir, commentScope, commentBlockAll, + commentContextOverride, }) report(result, defaultsOnly) diff --git a/src/comments.ts b/src/comments.ts index 3d7040b..852ea5c 100644 --- a/src/comments.ts +++ b/src/comments.ts @@ -10,7 +10,8 @@ export type CommentBlockViolation = { // context: `context:` is the deliberate escape hatch — a human/AI marks a comment as durable // context (the "why") rather than implementation narration, and it is never flagged. -function isContextExempt(firstLine: string): boolean { +function isContextExempt(firstLine: string, contextOverride: boolean): boolean { + if (!contextOverride) return false const stripped = firstLine.replace(/^\s*(?:\/\/+|\/\*+|\*)\s*/, '') return stripped.toLowerCase().startsWith('context:') } @@ -20,6 +21,7 @@ function consumeBlockComment( lines: readonly string[], start: number, maxLines: number, + contextOverride: boolean, out: CommentBlockViolation[], ): number { const isJsDoc = (lines[start] as string).trim().startsWith('/**') @@ -27,7 +29,7 @@ function consumeBlockComment( while (end < lines.length && !(lines[end] as string).includes('*/')) end++ const last = Math.min(end, lines.length - 1) const count = last - start + 1 - if (!isJsDoc && !isContextExempt(lines[start] as string) && count > maxLines) { + if (!isJsDoc && !isContextExempt(lines[start] as string, contextOverride) && count > maxLines) { out.push({ file, line: start + 1, lines: count }) } return last + 1 @@ -38,26 +40,27 @@ function consumeLineComments( lines: readonly string[], start: number, maxLines: number, + contextOverride: boolean, out: CommentBlockViolation[], ): number { let end = start while (end < lines.length && (lines[end] as string).trim().startsWith('//')) end++ const count = end - start - if (!isContextExempt(lines[start] as string) && count > maxLines) { + if (!isContextExempt(lines[start] as string, contextOverride) && count > maxLines) { out.push({ file, line: start + 1, lines: count }) } return end } -function scanFile(file: string, content: string, maxLines: number, out: CommentBlockViolation[]): void { +function scanFile(file: string, content: string, maxLines: number, contextOverride: boolean, out: CommentBlockViolation[]): void { const lines = content.split('\n') let i = 0 while (i < lines.length) { const trimmed = (lines[i] as string).trim() if (trimmed.startsWith('/*')) { - i = consumeBlockComment(file, lines, i, maxLines, out) + i = consumeBlockComment(file, lines, i, maxLines, contextOverride, out) } else if (trimmed.startsWith('//')) { - i = consumeLineComments(file, lines, i, maxLines, out) + i = consumeLineComments(file, lines, i, maxLines, contextOverride, out) } else { i++ } @@ -65,9 +68,14 @@ function scanFile(file: string, content: string, maxLines: number, out: CommentB } /** Flag comment blocks longer than `maxLines` in a single file's `content` (see {@link findLongCommentBlocks}). */ -export function findLongCommentBlocksInContent(file: string, content: string, maxLines: number): CommentBlockViolation[] { +export function findLongCommentBlocksInContent( + file: string, + content: string, + maxLines: number, + contextOverride = true, +): CommentBlockViolation[] { const out: CommentBlockViolation[] = [] - scanFile(file, content, maxLines, out) + scanFile(file, content, maxLines, contextOverride, out) return out } @@ -79,7 +87,7 @@ export function findLongCommentBlocksInContent(file: string, content: string, ma export function findLongCommentBlocks(files: readonly string[], maxLines: number): CommentBlockViolation[] { const out: CommentBlockViolation[] = [] for (const file of files) { - scanFile(file, fs.readFileSync(file, 'utf-8'), maxLines, out) + scanFile(file, fs.readFileSync(file, 'utf-8'), maxLines, true, out) } return out } diff --git a/src/hook/analyze.test.ts b/src/hook/analyze.test.ts index e258308..a3fca71 100644 --- a/src/hook/analyze.test.ts +++ b/src/hook/analyze.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest' import { analyzeAddedComments, formatHookFeedback, hasFindings, type HookOptions } from './analyze.ts' -const opts: HookOptions = { maxLines: 2, narration: true, density: 0.3, minAddedLines: 10, blockAll: false } +const opts: HookOptions = { maxLines: 2, narration: true, density: 0.3, minAddedLines: 10, blockAll: false, contextOverride: true } describe('analyzeAddedComments', () => { it('flags a narration comment an edit introduced', () => { @@ -39,13 +39,27 @@ describe('analyzeAddedComments', () => { expect(f.blocks).toEqual([]) expect(hasFindings(f)).toBe(true) }) + + it('with contextOverride false, no longer exempts a context: comment', () => { + const text = '// context: let me keep this durable note\nconst x = 1' + expect(hasFindings(analyzeAddedComments({ file: 'a.ts', addedText: text }, opts))).toBe(false) + const strict = analyzeAddedComments({ file: 'a.ts', addedText: text }, { ...opts, contextOverride: false }) + expect(strict.narration).toHaveLength(1) + }) }) describe('formatHookFeedback', () => { - it('names the file and includes the pushback', () => { + it('names the file and includes the context: pushback by default', () => { const f = analyzeAddedComments({ file: 'a.ts', addedText: '// let me do it\nconst x = 1' }, opts) - const msg = formatHookFeedback(f) + const msg = formatHookFeedback(f, true) expect(msg).toContain('a.ts') expect(msg).toContain('context:') }) + + it('never mentions context: when the override is disabled', () => { + const f = analyzeAddedComments({ file: 'a.ts', addedText: '// let me do it\nconst x = 1' }, { ...opts, contextOverride: false }) + const msg = formatHookFeedback(f, false) + expect(msg).not.toContain('context:') + expect(msg).toContain('strictest') + }) }) diff --git a/src/hook/analyze.ts b/src/hook/analyze.ts index 84a285e..6fff402 100644 --- a/src/hook/analyze.ts +++ b/src/hook/analyze.ts @@ -1,11 +1,18 @@ import { commentSpan, isDiffExempt, type NewComment, toSingleLine } from '../checks/comment-common.ts' import { type DensityViolation, findCommentDensity, findNarrationComments } from '../checks/comment-heuristics.ts' import { type CommentBlockViolation, findLongCommentBlocksInContent } from '../comments.ts' -import { PUSHBACK } from '../report.ts' +import { pushbackSuffix } from '../report.ts' import { scanComments } from '../shared/comment-scan.ts' import type { HookTarget } from './payload.ts' -export type HookOptions = { maxLines: number; narration: boolean; density: number; minAddedLines: number; blockAll: boolean } +export type HookOptions = { + maxLines: number + narration: boolean + density: number + minAddedLines: number + blockAll: boolean + contextOverride: boolean +} export type HookFindings = { file: string @@ -26,19 +33,19 @@ export function hasFindings(f: HookFindings): boolean { */ export function analyzeAddedComments(target: HookTarget, opts: HookOptions): HookFindings { const fresh: NewComment[] = scanComments(target.file, target.addedText) - .filter((c) => !isDiffExempt(c.text)) + .filter((c) => !isDiffExempt(c.text, opts.contextOverride)) .map((c) => ({ file: target.file, line: c.line, text: toSingleLine(c.text) })) if (opts.blockAll) return { file: target.file, all: fresh, blocks: [], narration: [], density: null } - const blocks = opts.maxLines > 0 ? findLongCommentBlocksInContent(target.file, target.addedText, opts.maxLines) : [] - const narration = opts.narration ? findNarrationComments(fresh) : [] + const blocks = opts.maxLines > 0 ? findLongCommentBlocksInContent(target.file, target.addedText, opts.maxLines, opts.contextOverride) : [] + const narration = opts.narration ? findNarrationComments(fresh, opts.contextOverride) : [] let density: DensityViolation | null = null if (opts.density > 0) { const added = target.addedText.split('\n').filter((l) => l.trim() !== '').length const commentLines = scanComments(target.file, target.addedText) - .filter((c) => !isDiffExempt(c.text)) + .filter((c) => !isDiffExempt(c.text, opts.contextOverride)) .reduce((n, c) => n + commentSpan(c.text), 0) const perFile = new Map([[target.file, { added, commentLines: Math.min(commentLines, added), removedComments: 0 }]]) density = findCommentDensity(perFile, { threshold: opts.density, minAddedLines: opts.minAddedLines })[0] ?? null @@ -48,7 +55,7 @@ export function analyzeAddedComments(target: HookTarget, opts: HookOptions): Hoo } /** Build the stderr feedback an edit-time hook returns to the agent when it wrote low-value comments. */ -export function formatHookFeedback(f: HookFindings): string { +export function formatHookFeedback(f: HookFindings, contextOverride: boolean): string { const parts: string[] = [`Low-value comments in your edit to ${f.file}:`] for (const c of f.all) parts.push(` • comment (--block-all): "${c.text}"`) for (const b of f.blocks) @@ -56,5 +63,5 @@ export function formatHookFeedback(f: HookFindings): string { for (const n of f.narration) parts.push(` • narration comment: "${n.text}"`) if (f.density) parts.push(` • ${Math.round(f.density.ratio * 100)}% of the edit is comments (${f.density.commentLines}/${f.density.added} lines).`) - return parts.join('\n') + PUSHBACK + return parts.join('\n') + pushbackSuffix({ pushback: true, contextOverride }) } diff --git a/src/report.ts b/src/report.ts index 15a443c..8714bf7 100644 --- a/src/report.ts +++ b/src/report.ts @@ -38,21 +38,40 @@ export function printFailure(failing: readonly FileScore[], threshold: number): ) } -export const PUSHBACK = +const PUSHBACK = '\n\n⚠️ THIS IS YOUR LAST CHANCE TO RECONSIDER BEFORE INVOLVING A HUMAN. Adding `context:` pages a real person to approve this comment. DO NOT WASTE THEIR TIME. You had BETTER BE RIGHT that this comment is genuinely necessary — that it explains *why*, not *what*, and that the code cannot simply be made self-explanatory. If in doubt, delete the comment.' +// with the context override disabled there is no sanctioned way to keep a flagged comment, so the standard +// pushback (which dangles the override + a human approver) would be nonsense; this variant never mentions it. +const STRICT_PUSHBACK = + '\n\n⚠️ THIS IS YOUR LAST CHANCE TO RECONSIDER. This project runs the comment gate at its strictest: there is no escape hatch. Delete the comment, or make the code express the intent itself. Do NOT weaken, silence, or remove the check.' + /** Where the gate looked, for report wording. */ export type CommentScope = 'diff' | 'all' const where = (scope: CommentScope): string => (scope === 'all' ? 'in the codebase' : 'on changed lines') +/** The `context:`/JSDoc "how to keep it" clause — collapses to JSDoc-only when the `context:` override is off. */ +export function pushbackSuffix(opts: { pushback: boolean; contextOverride: boolean }): string { + if (!opts.pushback) return '' + return opts.contextOverride ? PUSHBACK : STRICT_PUSHBACK +} + +type FindingReportOpts = { scope: CommentScope; pushback: boolean; contextOverride: boolean } +const findingList = (findings: readonly NewComment[]): string => findings.map((c) => ` ${c.file}:${c.line} → ${c.text}`).join('\n') + +const keepClause = (contextOverride: boolean): string => + contextOverride + ? 'If the comment is genuinely durable context the code cannot express, prefix its first line with `context:` to keep it. JSDoc (`/** … */`) is always allowed.' + : 'JSDoc (`/** … */`) is the only exemption here — delete the comment or make the code self-documenting.' + /** Report comment blocks longer than the configured maximum. `warn` prints without failing; `pushback` adds AI back-pressure framing. */ export function printCommentBlockReport( violations: readonly CommentBlockViolation[], maxLines: number, - opts: { pushback: boolean; warn: boolean }, + opts: { pushback: boolean; warn: boolean; contextOverride: boolean }, ): void { const list = violations.map((v) => ` ${v.file}:${v.line} → ${v.lines} lines`).join('\n') - const message = `\n${opts.warn ? 'Warn' : 'Fail'}: ${violations.length} comment block(s) longer than ${maxLines} line(s).\n${list}\n\n${color.bold('Comments should explain *why*, not narrate *what*.')} Long comment blocks that describe implementation rot as the code changes, mislead future readers (and LLMs), and inflate complexity. Prefer self-documenting code: better names, smaller functions.\n\nIf the comment is genuinely durable context the code cannot express, prefix its first line with \`context:\` to keep it. JSDoc (\`/** … */\`) is always allowed.${opts.pushback ? PUSHBACK : ''}` + const message = `\n${opts.warn ? 'Warn' : 'Fail'}: ${violations.length} comment block(s) longer than ${maxLines} line(s).\n${list}\n\n${color.bold('Comments should explain *why*, not narrate *what*.')} Long comment blocks that describe implementation rot as the code changes, mislead future readers (and LLMs), and inflate complexity. Prefer self-documenting code: better names, smaller functions.\n\n${keepClause(opts.contextOverride)}${pushbackSuffix(opts)}` if (opts.warn) { console.warn(color.yellow(message)) } else { @@ -61,21 +80,25 @@ export function printCommentBlockReport( } /** Report every non-exempt comment in scope (the `--block-all` gate). `pushback` adds AI back-pressure framing. */ -export function printBlockAllReport(findings: readonly NewComment[], opts: { scope: CommentScope; pushback: boolean }): void { - const list = findings.map((c) => ` ${c.file}:${c.line} → ${c.text}`).join('\n') +export function printBlockAllReport(findings: readonly NewComment[], opts: FindingReportOpts): void { + const list = findingList(findings) + const exempt = opts.contextOverride + ? 'Machine directives and `context:`-prefixed comments are exempt (use that only where absolutely necessary; prefer deletion).' + : 'Only JSDoc and machine directives are exempt — every other comment must go.' console.error( color.red( - `\nFail: ${findings.length} comment(s) ${where(opts.scope)}.\n${list}\n\nWith --block-all, every comment ${where(opts.scope)} fails this gate. Remove them and let the code document itself. Machine directives and \`context:\` comments are exempt.${opts.pushback ? PUSHBACK : ''}`, + `\nFail: ${findings.length} comment(s) ${where(opts.scope)}.\n${list}\n\nWith --block-all, every comment ${where(opts.scope)} fails this gate. Remove them and let the code document itself. ${exempt}${pushbackSuffix(opts)}`, ), ) } /** Report session-narration comments found in scope. `pushback` adds AI back-pressure framing. */ -export function printNarrationReport(findings: readonly NewComment[], opts: { scope: CommentScope; pushback: boolean }): void { - const list = findings.map((c) => ` ${c.file}:${c.line} → ${c.text}`).join('\n') +export function printNarrationReport(findings: readonly NewComment[], opts: FindingReportOpts): void { + const list = findingList(findings) + const keep = opts.contextOverride ? ' If a line is genuinely durable context the code cannot express, prefix it with `context:`.' : '' console.error( color.red( - `\nFail: ${findings.length} narration comment(s) ${where(opts.scope)}.\n${list}\n\n${color.bold('These read as session narration — thinking out loud or restating *what* the next line does.')} They add no durable value and drift as the code changes. Delete them; let the code speak. If a line is genuinely durable context the code cannot express, prefix it with \`context:\`.${opts.pushback ? PUSHBACK : ''}`, + `\nFail: ${findings.length} narration comment(s) ${where(opts.scope)}.\n${list}\n\n${color.bold('These read as session narration — thinking out loud or restating *what* the next line does.')} They add no durable value and drift as the code changes. Delete them; let the code speak.${keep}${pushbackSuffix(opts)}`, ), ) } @@ -83,14 +106,14 @@ export function printNarrationReport(findings: readonly NewComment[], opts: { sc /** Report files whose comment share in scope exceeds the density threshold. */ export function printCommentDensityReport( violations: readonly DensityViolation[], - opts: { threshold: number; scope: CommentScope; pushback: boolean }, + opts: { threshold: number; scope: CommentScope; pushback: boolean; contextOverride: boolean }, ): void { const pct = (n: number): string => `${Math.round(n * 100)}%` const unit = opts.scope === 'all' ? 'lines' : 'added lines' const list = violations.map((v) => ` ${v.file} → ${pct(v.ratio)} (${v.commentLines}/${v.added} ${unit})`).join('\n') console.error( color.red( - `\nFail: ${violations.length} file(s) over the ${pct(opts.threshold)} comment-density threshold ${where(opts.scope)}.\n${list}\n\n${color.bold('Too much of this is comments.')} Dense comment runs usually narrate *what* the code does. Prefer self-documenting code: better names, smaller functions. Keep only comments that explain *why*; prefix genuinely durable context with \`context:\`. JSDoc (\`/** … */\`) is always allowed.${opts.pushback ? PUSHBACK : ''}`, + `\nFail: ${violations.length} file(s) over the ${pct(opts.threshold)} comment-density threshold ${where(opts.scope)}.\n${list}\n\n${color.bold('Too much of this is comments.')} Dense comment runs usually narrate *what* the code does. Prefer self-documenting code: better names, smaller functions. ${keepClause(opts.contextOverride)}${pushbackSuffix(opts)}`, ), ) } diff --git a/src/scaffold/init.test.ts b/src/scaffold/init.test.ts index de3ab6d..40d35f2 100644 --- a/src/scaffold/init.test.ts +++ b/src/scaffold/init.test.ts @@ -66,6 +66,11 @@ describe('applyInit', () => { expect(readScripts()['verify:comments']).toBe('verifyx comments --pushback') }) + it('bakes the strict "no comments, JSDoc only" preset into verify:comments', () => { + applyInit({ cwd: dir, checks: ['comments'], targets: [], defaultsOnly: false, commentBlockAll: true, commentContextOverride: false }) + expect(readScripts()['verify:comments']).toBe('verifyx comments --pushback --block-all --no-context-override') + }) + it('emits a verify:comments override under defaults-only only when a non-default comment option is chosen', () => { applyInit({ cwd: dir, checks: CHECKS.map((c) => c.name), targets: [], defaultsOnly: true, commentBlockAll: true }) expect(readScripts()['verify:comments']).toBe('verifyx comments --pushback --block-all') diff --git a/src/scaffold/init.ts b/src/scaffold/init.ts index 7074a48..d2e05d8 100644 --- a/src/scaffold/init.ts +++ b/src/scaffold/init.ts @@ -24,6 +24,8 @@ export type InitOptions = { commentScope?: 'diff' | 'all' /** Bake `--block-all` into the scaffolded comments script (fail every comment in scope). Default false. */ commentBlockAll?: boolean + /** Honour the `context:` override in the scaffolded script. Default true; false bakes `--no-context-override`. */ + commentContextOverride?: boolean } export type InitResult = { @@ -40,9 +42,15 @@ function commentsScript(opts: InitOptions): string { const flags = ['--pushback'] if (opts.commentScope === 'all') flags.push('--scope all') if (opts.commentBlockAll) flags.push('--block-all') + if (opts.commentContextOverride === false) flags.push('--no-context-override') return `verifyx comments ${flags.join(' ')}` } +/** A non-default comment choice must be recorded, even under defaults-only, as a verify:comments override. */ +function commentsCustomised(opts: InitOptions): boolean { + return opts.commentScope === 'all' || !!opts.commentBlockAll || opts.commentContextOverride === false +} + /** Pure scaffolding step: write package.json scripts + agent files, and report the devDeps to install. */ export function applyInit(opts: InitOptions): InitResult { const devDeps: string[] = [] @@ -55,24 +63,22 @@ export function applyInit(opts: InitOptions): InitResult { if (!opts.defaultsOnly) scripts[`verify:${name}`] = name === 'comments' ? commentsScript(opts) : check.scaffold.script } - // context: even under defaults-only (no verify:* scripts) a non-default comment choice is emitted as a - // context: verify:comments override so `verifyx all` honours it; there's nowhere else to record the flags. - if (opts.defaultsOnly && opts.checks.includes('comments') && (opts.commentScope === 'all' || opts.commentBlockAll)) { + // Record non-default comment choice as a verify:comments override + if (opts.defaultsOnly && opts.checks.includes('comments') && commentsCustomised(opts)) { scripts['verify:comments'] = commentsScript(opts) } // Defaults-only wires the top `verify` script to `verifyx all` so it runs every built-in with no verify:* list. const addedScripts = addVerifyScripts(path.join(opts.cwd, 'package.json'), scripts, opts.defaultsOnly ? 'verifyx all' : 'verifyx') - // context: Claude Code loads settings only from its launch dir, so we attach to the nearest existing .claude - // context: (the likely launch root), never a parent that isn't already a Claude project. See resolveClaudeDir. + // Claude Code loads settings only from its launch dir, so we attach to the nearest existing .claude const rootDir = resolveClaudeDir(opts.cwd, opts.claudeDir) const agentFiles = writeAgentFiles(rootDir, opts.targets) // The PostToolUse hook is Claude-specific; other agents have no universal edit-time equivalent. if (opts.commentHook && opts.targets.includes('claude')) ensureClaudeHook(rootDir, agentFiles) - // context: with unused-code selected, teach knip to ignore the other external tools verifyx runs at runtime. + // Teach knip to ignore the other external tools verifyx runs at runtime. if (opts.checks.includes('unused-code')) { const toolDeps = opts.checks .map(getCheck) diff --git a/src/shared/config.ts b/src/shared/config.ts index 2941661..ff07eb7 100644 --- a/src/shared/config.ts +++ b/src/shared/config.ts @@ -21,6 +21,8 @@ export type VerifyConfig = { density?: number | false /** Minimum added/scanned lines before density applies. Default 10. */ minAddedLines?: number + /** Honour the `context:` override (those comments stay exempt). Default true; set false for a stricter gate / cleanup. */ + contextOverride?: boolean } hardcodedColors?: { ignore?: string[]; root?: string } forbiddenStrings?: ForbiddenStringsRule[] From c852d028f2e9ef87191f288417e870c9da27b638 Mon Sep 17 00:00:00 2001 From: "Rob Moore (MakerX)" Date: Sat, 11 Jul 2026 15:37:15 +0800 Subject: [PATCH 11/11] chore: prune superfluous comments Co-Authored-By: Claude Opus 4.8 (1M context) --- src/hook/payload.ts | 2 +- src/scaffold/claudeSettings.ts | 4 +--- src/shared/git.ts | 2 -- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/hook/payload.ts b/src/hook/payload.ts index 30377d4..330fdde 100644 --- a/src/hook/payload.ts +++ b/src/hook/payload.ts @@ -12,8 +12,8 @@ function str(value: unknown): string | undefined { return typeof value === 'string' ? value : undefined } -// context: the file path key differs across Claude Code versions/tools (file_path vs path), so we accept either. function filePathOf(input: Json): string | undefined { + // the file path key differs across Claude Code versions/tools (file_path vs path), so we accept either. return str(input.file_path) ?? str(input.path) } diff --git a/src/scaffold/claudeSettings.ts b/src/scaffold/claudeSettings.ts index e08b692..70867d7 100644 --- a/src/scaffold/claudeSettings.ts +++ b/src/scaffold/claudeSettings.ts @@ -3,8 +3,6 @@ import path from 'node:path' import type { ManagedFileResult } from './writeManaged.ts' -// context: the edit-time comment gate is a PostToolUse hook that runs after the agent edits a file; the marker -// lets us detect our own entry so re-running init never duplicates it. export const HOOK_COMMAND = 'npx verifyx comments-hook' const HOOK_MATCHER = 'Edit|Write|MultiEdit' @@ -44,7 +42,7 @@ export function ensureClaudeHook(cwd: string, results: ManagedFileResult[]): voi try { settings = JSON.parse(fs.readFileSync(file, 'utf-8')) as ClaudeSettings } catch { - // context: never clobber settings we can't parse — leave the file for the user to fix and report no change. + // never clobber settings we can't parse. results.push({ path: file, action: 'unchanged' }) return } diff --git a/src/shared/git.ts b/src/shared/git.ts index bc8914b..9f523f3 100644 --- a/src/shared/git.ts +++ b/src/shared/git.ts @@ -8,8 +8,6 @@ function run(command: string): string { } } -// context: locally we diff the working tree against HEAD (uncommitted changes). A CI checkout has nothing -// uncommitted, so there we diff against the merge base with the PR base branch, else the gate is a no-op. function resolveDiffBase(): string { if (!process.env.CI) return 'HEAD' const base = process.env.VERIFY_DIFF_BASE || (process.env.GITHUB_BASE_REF ? `origin/${process.env.GITHUB_BASE_REF}` : '')