Encoding Software Design Principles In Skills

## Definitions

**Interface comment**: a comment block that immediately precedes the declaration of a module such as a class, data structure, function, or method. The comment describes the module’s interface.

**Implementation comment**: a comment inside the code of a method or function, which describes how the code works internally.

## All Interface Comments Must Follow TSDoc

All interface comments should follow TSDoc conventions from `https://tsdoc.org/`.

In existing comments, flag:

- exported classes, functions, methods, interfaces, types, or consts documented with `//` instead of a TSDoc `/** ... */` block
- malformed TSDoc tags, especially `@param` without the `@param name - description` shape
- missing or malformed `@returns` on documented non-void returns
- `@remarks`, `@throws`, `@deprecated`, or `{@link ...}` content written in non-TSDoc form when those concepts are present
- long detail in the summary paragraph that should be moved under `@remarks`

Do not require every exported declaration to have a comment; absence of interface comments is out of scope. Do not flag inline implementation comments for not being TSDoc.

Example:

```ts
/**
 * Opens the payment session.
 *
 * @param request - The normalized checkout request.
 * @returns The provider URL the caller should redirect to.
 */
```

All interface comments that you propose should follow TSDoc.

## Check For Red Flags

For each comment in scope, look for the red flags below.

### Red Flag: Comment References Current Session

The comment references a bug that the current session is missing, or otherwise would not make sense without the context of the current conversation.

Suggested fix: delete

### Red Flag: Comment Repeats Code

The comment restates the adjacent identifier or tokens in English without adding information.

Detection signal: at least half of the comment's content words appear in the adjacent identifier or statement, and the comment contributes no units, invariants, intent, boundary behavior, or non-obvious behavior.

Example to flag:

```ts
// Get pointer copy
const ptrCopy = getCopy(obj);
```

Suggested fix: delete the comment or rewrite it with different words that add real information.

### Red Flag: Missing Precision

Interface comments lack non-obvious information that the type does not communicate:

- units
- inclusive or exclusive boundaries
- null, undefined, empty, or default behavior
- invariants the value always satisfies
- meaningful error or rejection behavior

Only flag when the signature and nearby code genuinely do not communicate the missing information.

### Red Flag: Interface Comment Leaks Implementation

An interface comment describes how the implementation works instead of what callers can rely on.

Indicators include references to private fields, private helpers, internal data structures, iteration strategy, recursion, storage layout, or phrases such as `internally`, `implemented as`, `uses a map`, or `loops through`.

When flagging this, append:

```text
If the abstraction genuinely cannot be described without these implementation details, this class/method may be shallow; consider whether the interface should hide more.
```

### Red Flag: Interface Coverage Gap

An existing interface comment is missing meaningful contract information:

- a useful `@param` for a non-obvious parameter
- `@returns` behavior for a non-void return
- side effects
- preconditions
- thrown errors or rejected promises when callers can reasonably handle them

Only flag non-trivial gaps. If a tag exists but merely repeats the parameter or return name, flag it under `Don't Repeat The Code`.

### Red Flag: Implementation Comment Describes The Line Below

A single-line `//` comment immediately above a statement restates that statement without explaining why.

Example to flag:

```ts
// Add a horizontal scroll bar
add(new JScrollBar(JScrollBar.HORIZONTAL));
```

Suggested fix: delete the comment or replace it with the reason the statement exists.

### Red Flag: Cross-Module Decision

Flag comments that reveal a decision belongs somewhere central instead of being scattered across modules.

Primary signal: phrases such as `must match`, `in sync with`, `see also`, `consistent with`, `this assumes`, `mirrors`, or `matches the X in Y`.

Secondary signal: after reading all selected files, surface repeated non-local assumptions that clearly appear in comments across multiple reviewed files.

Suggested fix: Find a way to centralize the decision in code (e.g., pull out logic into a shared function) or in a document (like an ADR).

## Propose New Comments

Look for these opportunities in code that has no comments and code that already has comments. If a comment already exists but could take advantage of one or more of these opportunities, propose a change to the comment.

### Opportunity: Provide Higher-Level Intuition

Flag a code block (a function or a part of a function) for intuition-enhancing:

- The block contains complex logic (e.g., loops, nested conditionals, or async/await)
- The block contains many dependencies from other files
- The block is a long function (greater than 20 lines)
- The block may otherwise be hard to read for a developer new to the codebase

Then, write a high-level comment that enhances intuition, omitting details and helping the reader understand the overall intent and structure of the code.

#### Example

```ts
if (numProcessedPKHashes < readRpc[i].numHashes) {
  // Some of the key hashes couldn't be looked up in
  // this request (either because they aren't stored
  // on the server, the server crashed, or there
  // wasn't enough space in the response message).
  // Mark the unprocessed hashes so they will get
  // reassigned to new RPCs.
  for (let p = removePos; p < insertPos; p++) {
    if (activeRpcId[p] === i) {
      if (numProcessedPKHashes > 0) {
        numProcessedPKHashes--;
      } else {
        if (p < assignPos) {
          assignPos = p;
        }
        activeRpcId[p] = RPC_ID_NOT_ASSIGNED;
      }
    }
  }
}
```

### Opportunity: Provide Precision

Look for function arguments and return values where the name, type, and existing comment lack important details. For example:

- What are the units for this variable?
- Are the boundary conditions inclusive or exclusive?
- If a null value is permitted, what does it imply?
- Are there certain properties that are always true for the variable (invariants), such as “this list always contains at least one entry”?

Provide a comment that clarifies the meaning of the code and adds missing details.

When documenting a variable, think nouns, not verbs. In other words, focus on what the variable represents, not how it is manipulated.

#### Example

```ts
/* True means that a heartbeat has been received since the last time
 * the election timer was reset. Used for communication between the
 * Receiver and PeriodicTasks threads. */
private receivedValidHeartbeat: boolean;
```

# Comments

Do not write comments that only make sense in the context of our conversation.

By default, comments agents write should satisfy the `/comments-review` standard:

**Do**

- Write comments when they clarify non-obvious contracts, invariants, units, boundary behavior, side effects, error behavior, or the high-level intent of complex code.
- Use TSDoc `/** ... */` blocks and standard tags such as `@param`, `@returns`, `@throws`, `@deprecated`, and `{@link ...}` when those details are present.

**Do Not**

- Write comments that are obvious from the code.
- Write comments that merely restate the adjacent identifier, statement, or control flow.
- Leak private helpers, storage details, iteration strategy, or other implementation details from interface comments.
- Write comments that reference our conversation or the bug we're fixing.