Skip to content
Merged
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: 2 additions & 0 deletions packages/agents/content/skills/_data/comment-discipline.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ Explain only why this specific lint rule is being suppressed here. Not the surro
Good: `// eslint-disable-next-line no-explicit-any -- third-party Stripe type ships as any.`
Bad: `// eslint-disable-next-line no-explicit-any -- this test locks in the affordance for users with screen readers, see PR #456 for context.`

This carve-out governs the _comment_ — it permits keeping a tight rationale. It does not endorse the suppression itself; whether a suppression is justified at all is an `anti-patterns` question.

## Economy

Lines of code are lines of code, even when they are comments. They have to be maintained, scanned during review, and trusted as accurate. Every comment must earn its weight against that cost.
Expand Down
19 changes: 19 additions & 0 deletions packages/agents/content/skills/anti-patterns/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,25 @@ Common but problematic approaches that should be avoided in favor of proper solu
- **Don't accept "good enough" solutions** - If it creates maintenance burden, it's not good enough
- **Avoid band-aid fixes** - Address root causes rather than symptoms

### Suppression directives

- **A suppression directive — a comment that tells a linter or type checker to ignore a specific warning — is a design signal, not a workaround.** The rule flagged the code because it matched a pattern that is usually wrong.
- **Try these in order before suppressing inline:** (1) change the code so the rule no longer triggers — usually the right answer; (2) reconfigure the rule if its default is wrong for the project; (3) define a scoped exception for a whole category that is legitimately exempt. Suppress inline only as a last resort — when the case is genuinely local and none of the above fits, such as an external boundary where the real type can't be known.
- **Every suppression you introduce carries a rationale** naming the rule, why it doesn't apply here, and what alternatives you rejected. Restating the rule is not a rationale.

```ts
// ❌ Suppress the warning
// eslint-disable-next-line complexity -- does a lot
function classify(order) {
/* long, deeply nested */
}

// ✅ Change the code so the rule no longer triggers
function classify(order) {
return isExpedited(order) ? classifyExpedited(order) : classifyStandard(order);
}
```

### Error handling

- **Don't place non-failing code inside try/catch blocks** - Only wrap operations that can actually throw errors
Expand Down
1 change: 1 addition & 0 deletions packages/agents/content/skills/review-criteria/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Evaluation criteria for code review. Apply proportionally — match depth to ris
## Skip

- Lint or formatting issues (automated tools handle these) — e.g., unused imports, missing semicolons, import order, whitespace. Do not create findings for issues that CI linters will catch; at most mention them in passing prose.
- Author-introduced suppression directives are in scope. A new lint/type suppression added in this change defaults to a Warning (⚠️) unless its rationale demonstrates a legitimate carve-out (see `anti-patterns`). Pre-existing suppressions in unchanged code remain Legacy.
- Pre-existing issues in unchanged code (categorize as Legacy if noted)

## Logic verification
Expand Down
Loading