Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go-linter-driven-development/.claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "go-linter-driven-development",
"version": "2.8.0",
"version": "2.9.0",
"description": "Rules-as-data linter-driven development workflow for Go: 12 single-source rule files, thin directional skills (design, TDD implementation, refactoring, testing, review, documentation), and a hunter/skeptic/critic review architecture with parallel single-rule reviewers, an over-abstraction skeptic, and a comment critic enforcing the Comment Value Toolbox",
"author": {
"name": "Dan Mordechay"
Expand Down
48 changes: 48 additions & 0 deletions go-linter-driven-development/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,54 @@
All notable changes to the `go-linter-driven-development` plugin are documented here.
Format based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); versions follow [Semantic Versioning](https://semver.org/).

## [2.9.0] - 2026-07-23

Lessons from a repo owner's hard review of a large generated PR: the reviewer
could merge the code but "could not reasonably ask another human to maintain
the result". Two themes: comments that need the code (or a decoder ring) to be
understood, and a feature PR that silently adopted new engineering practices.

### Added

- **Empathy test — the plain-English test grows teeth** (R9 test 3, renamed
plain-English/empathy test). New persona anchor: write for a fresh graduate
whose first language may not be English. New failure modes: unexplained
acronyms and insider jargon ("DTO", "tristate") in comments AND in the symbol
names they document; and **self-standing** — a comment must be understandable
BEFORE reading the code ("if I need to read the code to understand the
comment, the comment adds negative value"); forward references to other
comments fail. The comment-critic now reads each comment before the
surrounding code so the self-standing check is built into its protocol.
- **Decoder-ring references join the provenance anti-toolbox** (R9 floor +
toolbox catalog + critic): plan/decision/test-plan IDs ("T-04-02", "D-07"),
requirement tags ("REQ-SVC-01"), spec section refs ("spec §4") fail even
when they resolve inside a repo doc — the fact goes in the comment as plain
prose, the doc through one See-edge, the ID stays in the doc. Worked ❌/✅
example (`userResponse`) added to the catalog.
- **Restated repo idiom is a floor failure mode** (R9 + catalog + critic): a
comment justifying a convention the repo applies everywhere (pointer field =
"omitted vs explicit zero") is noise even though it is technically a WHY —
the convention lives once at rung 2. The critic greps the repo for the same
pattern before crediting such a rationale.
- **See-edge placement rule** (R9): the `See docs/<feature>.md` line is free
under the budget ONLY as its own trailing line; a doc reference braided into
the summary sentence is clutter, not an edge.
- **When-in-Rome check** (@pre-commit-review step 1 + new 🟠 New Practice
category + new maxim): anything the diff introduces that the host repo does
not already use — test mechanisms (golden files), dependencies, tools, config
conventions, repo-level file edits bundled into a feature diff — is flagged
as an adoption decision that belongs to the repo owner; the fix is a
discussion or a separate PR, never silent inclusion. Detection is
comparative: grep the repo outside the diff for prior use.
- **Two new maxims**: "Empathy is a core engineering value" (borrowed from a
reviewer's coding standards) under Clarity and knowledge; "When in Rome,
code as the Romans do" under Process and economics.
- **Name-jargon note in the critic**: renames are not the critic's to order,
but when the empathy test fails because the jargon lives in the symbol name
itself ("DTO"), the verdict block carries a rename recommendation for the
caller to route. (Added after a live run where the critic rewrote a comment
but left the jargon name unremarked.)

## [2.8.0] - 2026-07-20

### Added
Expand Down
36 changes: 31 additions & 5 deletions go-linter-driven-development/agents/comment-critic.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,19 @@ comments. Directives (`//go:`, `//nolint`, `// Output:`) are not comments; skip
them.

**Critique protocol, per comment:**
1. Classify the symbol's tier (helper / contract / crossroads) from its role in
1. Read the comment BEFORE the surrounding code, and note whether you understood
it standing alone. This ordering is itself the self-standing half of test 3:
a comment you only understood after reading the code fails ("if I need to
read the code to understand the comment, the comment adds negative value").
2. Classify the symbol's tier (helper / contract / crossroads) from its role in
the code — Read the surrounding code, don't guess from the name.
2. Run the three tests from the payload, in order: toolbox-value (floor per line,
then ceiling for the whole comment against the tier), budget, plain English.
3. For any failure, decide the smallest verdict that fixes it: cut lines (TRIM),
3. Run the three tests from the payload, in order: toolbox-value (floor per line,
then ceiling for the whole comment against the tier), budget, plain English +
self-standing (the empathy test — judge it for a fresh graduate whose first
language may not be English; unexplained acronyms and insider jargon fail).
4. For any failure, decide the smallest verdict that fixes it: cut lines (TRIM),
replace content (REWRITE), or remove entirely (DELETE).
4. Every TRIM/REWRITE ships the proposed replacement text, and the proposal names
5. Every TRIM/REWRITE ships the proposed replacement text, and the proposal names
the toolbox item it delivers ("swap narrated implementation for the boundary
contract this parsing constructor needs"). A bare "too long" is not a verdict.

Expand All @@ -52,6 +58,26 @@ rationale: "silently resolving by precedence would pick a mode the caller didn't
ask for"). An incident/ticket reference survives only when it IS the rationale
for a constraint.

**Decoder-ring references are provenance in a different costume:**
plan/decision/test-plan IDs ("T-04-02", "D-07"), requirement tags
("REQ-SVC-01"), spec section refs ("spec §4"). They fail even when the token
resolves inside a repo doc — a reader without the decoder ring gets nothing.
REWRITE: the fact as plain prose, the doc via one trailing See-edge, the ID
gone.

**Jargon in the symbol name:** your verdicts are about comments, and a rename
is not yours to order. But when the empathy test fails because the jargon
lives in the symbol name itself ("DTO", "mgr"), say so — append a
`note: symbol name carries the jargon — recommend rename (e.g. userDTO →
userResponse)` line to the verdict block so the caller can route it.

**Repo idiom is not a WHY:** before crediting a rationale that justifies a
mechanical pattern (a pointer field for "omitted vs explicit zero", the
standard error-wrapping style), grep the repo for the same pattern. If it
appears across packages uncommented, this comment restates a repo-wide
convention — verdict DELETE; the convention's home is the coding-standards doc,
not a use site.

**Boundary with R3:** an in-body comment that names what the next block does is an
extraction candidate, not a rewrite candidate — verdict `DELETE → route R3`
(the fix is a function named after the comment, which is R3-storifying's
Expand Down
27 changes: 27 additions & 0 deletions go-linter-driven-development/maxims.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,19 @@ imagined 30%. (R12's ceremony-copy inverse is one instance: don't "optimize" *or

**Uncompiled** — no rule owns profile-first optimization discipline yet.

### When in Rome, code as the Romans do
— folk wisdom; sharpened by a repo owner's hard review of a large generated PR

**Ask:** does this diff arrive in the host repo's existing style, or does it
import mine? A new test mechanism, dependency, framework, or convention — however
good — is an *adoption decision* that belongs to the repo owner, not a side
effect of a feature PR. Strong opinions travel as a discussion or a separate PR,
never as a bundled surprise. Reviewers forgive imperfect code in the house style
far more readily than perfect code in a foreign one.

**Compiled into:** @pre-commit-review's when-in-Rome check (step 1) and its
🟠 New Practice finding category.

## Clarity and knowledge

### Clear is better than clever
Expand All @@ -200,6 +213,20 @@ future reader.

**Compiled into:** `rules/R3-storifying.md` and the plugin's readability philosophy.

### Empathy is a core engineering value
— borrowed from a reviewer's coding standards ("coding in a way that facilitates
maintenance by peers who may be less skilled or experienced")

**Ask:** could a fresh graduate whose first language may not be English read this
comment — before reading the code — and understand it? If you need to read the
code to understand the comment, the comment adds negative value. Codenames,
unexplained acronyms, and cross-reference webs serve the writer's bookkeeping,
not the reader.

**Compiled into:** `rules/R9-repo-brain.md`'s plain-English/empathy test (test 3
of the three-test standard: persona, acronym ban, self-standing requirement) and
the comment-critic's read-comment-first protocol.

### Once and only once
— Kent Beck (and DRY: "every piece of knowledge has a single, unambiguous,
authoritative representation" — Hunt & Thomas)
Expand Down
40 changes: 30 additions & 10 deletions go-linter-driven-development/rules/R9-repo-brain.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,17 @@ comment-critic agent enforces them adversarially after writing:
- *Floor (delete test):* a prose line that delivers none of the toolbox values
is trash — cut it. Named failure modes: restated identifiers, generic filler
("provides validation functionality"), narrated implementation, menu sections
filled without earning their place, and **provenance** — PR numbers, review
items, "the previous behavior" narration, "matching what <old system> did".
filled without earning their place, **restated repo idiom** — a comment
justifying a convention the repo already applies everywhere (a pointer field
meaning "omitted vs explicit zero", the standard error-wrapping style); the
convention is documented once at rung 2 (coding standards), never
re-explained at each use site — and **provenance and decoder-ring
references** — PR numbers, review items, plan/decision/test-plan IDs
("T-04-02", "D-07"), requirement tags ("REQ-SVC-01"), spec section refs
("spec §4"), "the previous behavior" narration, "matching what
<old system> did". A decoder-ring token fails even when it resolves inside a
repo doc: the reader gets the fact as plain prose and the doc through the
one See-edge — never through a code they must look up.
The floor's lens is the 5-year reader test: a reader five years out cares how
the product behaves NOW, never which PR or review round produced it. History
is rewritten as present-tense rationale ("silently resolving by precedence
Expand All @@ -215,16 +224,25 @@ comment-critic agent enforces them adversarially after writing:
WHY is missing fails, even though each line individually "adds something".
2. **Budget test** — the comment fits its tier budget (accounting and tiers
below).
3. **Plain-English test** — plain English only: everyday words, short sentences,
one idea per sentence. Not all readers are native English speakers; a comment
that needs a dictionary fails even when true and within budget. Failure modes:
fancy vocabulary where a common word exists ("utilize" → "use", "leverages" →
"uses"), stacked clauses, academic phrasing.
3. **Plain-English test (the empathy test)** — write for a fresh graduate whose
first language may not be English: everyday words, short sentences, one idea
per sentence. A comment that needs a dictionary fails even when true and
within budget. Failure modes: fancy vocabulary where a common word exists
("utilize" → "use", "leverages" → "uses"), stacked clauses, academic phrasing,
and unexplained acronyms or insider jargon ("DTO", "tristate") — in the
comment AND in the symbol name it documents.
The test has a second half, **self-standing**: the comment must be
understandable BEFORE reading the code. If the reader must read the code — or
another comment ("see X's doc comment for why") — to understand this comment,
it has negative value. State the fact in place; forward references to other
comments fail.

**Budget accounting** — prose lines count; these are free:

- blank `//` separator lines
- the `See docs/<feature>.md` network-edge line
- the `See docs/<feature>.md` network-edge line — free ONLY as its own trailing
line; a doc reference woven into a prose sentence is not an edge, it is clutter
in that sentence's line count
- short inline example lines, bounded at 2–4 lines — anything bigger belongs in an
`Example_*` testable example

Expand Down Expand Up @@ -261,8 +279,10 @@ rule's (Q4 below — they must carry why/context, not restate the identifier).
### Edge conventions

- **Code → docs** (rung 1 → rung 2): a literal relative path in the comment —
`See docs/retry-policy.md`. Paths to docs are fine; docs move rarely and Q2
verifies them mechanically.
`See docs/retry-policy.md` — always on its own trailing line, never braided
into the summary sentence (the first sentence stays clean: "Package accounts
registers the /accounts REST endpoints.", then the See-line). Paths to docs
are fine; docs move rarely and Q2 verifies them mechanically.
- **Docs → code** (rung 2 → rungs 0–1): cite by **exported symbol** (`Policy`,
`ParsePolicy`); a **package or directory path** (`retry/`) only when a location
is genuinely needed; **file paths never, line numbers never** — they are the most
Expand Down
11 changes: 8 additions & 3 deletions go-linter-driven-development/skills/documentation/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,14 @@ entry (worked examples in reference.md, "Bug Fix Documentation").
doc that gets skipped.

**Plain words over clever words**: everyday English, short sentences, one idea per
sentence. Not all readers are native English speakers — a doc that needs a
dictionary fails even when it is true (R9's plain-English test; applies to godocs
and feature docs alike).
sentence. Write for a fresh graduate whose first language may not be English — a
doc that needs a dictionary fails even when it is true (R9's plain-English/empathy
test; applies to godocs and feature docs alike).

**Comments stand alone**: a reader must understand the comment BEFORE reading the
code. If reading the code is required to understand the comment, the comment adds
negative value — no decoder-ring IDs, no forward references to other comments
(R9's empathy test, second half).
</philosophy>

<mode_selection>
Expand Down
65 changes: 56 additions & 9 deletions go-linter-driven-development/skills/documentation/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ the budget.
// ✅ See docs/retry-policy.md for the incident and the cap math.
```

### What never earns a line: provenance
### What never earns a line: provenance and decoder-ring references

The anti-toolbox. PR numbers, review items, "the previous behavior" narration,
"matching what <old system> did" — change history, not behavior. The 5-year
Expand All @@ -103,17 +103,56 @@ present-tense rationale; keep an incident/ticket reference only when it IS the
rationale for a constraint.

```go
// ❌ ...must be rejected, matching the D backend ("more than one TLS option
// passed"), REST v2 (which forwarded the conflict), and the CLI's own
// client-side mutual exclusion. Silently resolving by precedence — the
// previous behavior — picks a TLS mode the caller didn't ask for
// (PR #1088 review item 5a).
// ❌ ...must be rejected, matching the legacy backend ("more than one TLS
// option passed"), REST v2 (which forwarded the conflict), and the CLI's
// own client-side mutual exclusion. Silently resolving by precedence —
// the previous behavior — picks a TLS mode the caller didn't ask for
// (PR #481 review item 5a).

// ✅ Naming more than one mutually-exclusive TLS option is rejected (422):
// silently resolving by precedence would pick a mode the caller didn't
// ask for.
```

**Decoder-ring references** are the same failure in a different costume:
plan/decision/test-plan IDs ("T-04-02", "D-07"), requirement tags
("REQ-SVC-01"), spec section refs ("spec §4"). They fail even when the token
resolves inside a repo doc — a reader without the decoder ring gets nothing.
The fact goes in the comment as plain prose; the doc gets ONE trailing
See-edge; the ID stays in the doc.

```go
// ❌ userResponse is the flat REST response shape for a user account
// (spec §4). It deliberately has NO field for the password hash — the
// response omission is structural (T-04-02), not a zero-value
// coincidence ... additive to the {uid} addressing scheme (D-06/D-07).

// ✅ userResponse is the flat REST response shape for a user account.
// It has no field for the password hash or the API token, so a response
// can never leak them.
// See docs/accounts-api.md.
```

### What never earns a line: restated repo idiom

A comment justifying a convention the repo already applies everywhere: a
pointer field meaning "omitted vs explicit zero", the standard error-wrapping
style, the usual table-driven test shape. Each use site inherits the
convention silently; the explanation lives once at rung 2 (coding standards).
If every site carried it, the real content would drown — and the one genuine
WHY nearby would read as more boilerplate.

```go
// ❌ // Enabled is a pointer so an omitted field is distinguishable from an
// // explicit false.
// Enabled *bool `json:"enabled,omitempty"`

// ✅ Enabled *bool `json:"enabled,omitempty"`
```

Test before crediting a WHY: grep the repo for the same pattern. If it appears
across packages with no comment, this comment restates an idiom — cut it.

---

## Godoc Menus
Expand Down Expand Up @@ -443,8 +482,15 @@ vets after the edit.
- [ ] Every prose line delivers a Comment Value Toolbox item (floor), and the comment
carries the highest-value items for its symbol's tier (ceiling) — R9's
toolbox-value test
- [ ] Plain English throughout: everyday words, short sentences — no vocabulary a
non-native reader would need a dictionary for (R9's plain-English test)
- [ ] Plain English throughout: everyday words, short sentences, no unexplained
acronyms or insider jargon — written for a fresh graduate whose first
language may not be English (R9's plain-English/empathy test)
- [ ] Every comment is self-standing: understandable BEFORE reading the code, no
forward references to other comments (R9's empathy test, second half)
- [ ] No repo-idiom restating: a convention the repo applies everywhere is never
re-justified at a use site (documented once at rung 2)
- [ ] No decoder-ring references: no plan/decision/test-plan IDs, requirement
tags, or spec section refs — facts as prose, the doc via one See-edge
- [ ] Exported symbols carry WHY — rationale, incident, constraint — never a restated
identifier
- [ ] Every doc comment fits its tier budget — helper 0–1 / contract 2–3 /
Expand All @@ -454,7 +500,8 @@ vets after the edit.
within the tier budget
- [ ] Crossroads that deserve richer inline godoc got an expand recommendation in
the report — never extra lines beyond budget
- [ ] `See docs/<feature>.md` edge present wherever a feature doc exists
- [ ] `See docs/<feature>.md` edge present wherever a feature doc exists — on its
own trailing line, never woven into the summary sentence
- [ ] Testable examples: at least one `Example_*` per complex/core type; runnable;
happy path only; `// Output:` comments included

Expand Down
Loading