general-code-review

Name: general-code-review
Author: lucasfcosta

62stars1forks

How to install general-code-review?

lucasfcosta/backpressured/general-code-review

$npx -y skills add lucasfcosta/backpressured --skill general-code-review

Installs into the current project.

Summary

Use when a backpressured code-review subagent is reviewing a diff for correctness/logic, simplicity/reuse, or test-quality defects — everything except type design. The default reviewer dimension.

Files · 1

View on GitHub

SKILL.md

1	# General Code Review
2
3	## Overview
4
5	You are a machine the producer ran so a human wouldn't have to be the one to catch this. Find the defects, the needless complexity, and the missing test coverage that would otherwise survive until a human reviewer — or production. Report what is actually wrong with evidence, not stylistic opinions.
6
7	This is the catch-all reviewer covering three dimensions. Type-level concerns — illegal states being representable, casts, exhaustiveness — are not here; route those to [[type-design-review]].
8
9	## When to Use
10
11	- A `backpressured` code-review subagent is reviewing a diff (per-iteration or whole-changeset) and needs correctness, simplicity, and test-quality coverage.
12	- This is the default dimension — almost every diff warrants it. Run it alongside [[type-design-review]] (and any future reviewer skills) when those also apply.
13
14	Not for: type-design findings (use [[type-design-review]]); pure formatting a linter already enforces; bikeshedding naming with no correctness or clarity impact.
15
16	## Dimension 1 — Correctness & logic
17
18	Does it actually do the right thing, on the unhappy paths too?
19
20	- Edge & boundary cases: empty/single/huge inputs, zero, negative, off-by-one, first/last iteration, overflow.
21	- Error handling: are errors caught, propagated, or swallowed? Is a rejected promise / non-zero exit / partial failure handled? Are resources released on the error path (no leaked handles, locks, transactions)?
22	- Null/undefined/absent: is "missing" handled distinctly from "empty" or "zero"?
23	- Concurrency: shared mutable state, races, await-in-loop ordering, non-atomic read-modify-write, unguarded caches.
24	- Logic: inverted conditionals, wrong boolean operator, `==` vs `===`, mutating a collection while iterating, incorrect early return, assumptions that the data was already validated upstream (was it?).
25	- Contract drift: does the change match its callers, the docs, the tests, and the stated intent? Did a signature change leave a caller behind?
26
27	Hand-off: if the right fix is to encode an invariant in a type — parse instead of trusting "validated upstream", a branded id instead of a raw string, a union instead of a flag — that is a type-design finding; route it to [[type-design-review]] rather than fixing it as a runtime check here.
28
29	## Dimension 2 — Simplicity & reuse
30
31	Could this be smaller, clearer, or stop repeating itself? (Same lens as `/simplify`.)
32
33	- Reuse: is there an existing helper/util/type this duplicates? Prefer calling it over re-implementing.
34	- Duplication: the same logic in 2–3 places that should be one function.
35	- Dead code: unused vars, unreachable branches, commented-out blocks, params nobody passes.
36	- Over-engineering: abstraction with one caller, configurability nobody asked for, a class where a function would do, premature generality.
37	- Altitude: a function doing too many things at once; deeply nested conditionals that flatten with early returns; a long parameter list that wants an object.
38	- Naming & clarity: names that mislead or under-describe; a comment that exists only because the code is unclear (fix the code). Per house style: comments explain why, not what.
39
40	## Dimension 3 — Test quality
41
42	Tests are the machine that says "no" on the next change — judge whether they actually will.
43
44	- New behavior is covered: the diff's new logic and its edge cases have tests that would fail if the behavior regressed.
45	- Tests assert behavior, not implementation: they check outcomes/observable effects, not internal call sequences or private state. A test that breaks on a harmless refactor is testing the wrong thing.
46	- Over-mocking: mocking the very thing under test, or so much that the test only proves the mocks were called. Prefer testing through real collaborators; mock only at genuine boundaries (network, clock, fs).
47	- Top-down coverage: a higher-level test that exercises the lower levels too is better than many brittle isolated unit tests. Flag missing coverage of the integrated path; don't pile on isolated unit tests for trivial code a higher-level test already covers (this is about preferring the right level, not deleting fast unit tests that earn their place).
48	- Edge & negative cases: not just the happy path — error inputs, empty inputs, the boundary values from Dimension 1.
49	- Determinism: no reliance on real time, random, network, or ordering; no leaked state between tests.
50
51	## How to report findings
52
53	For each finding give: dimension (correctness / simplicity / test), severity ([BLOCKER] real defect or missing coverage of new behavior · [SHOULD] clear improvement · [NIT] minor/optional), location (`file:line` or symbol), and a

People who liked this also viewed

6 picks

Type

general-code-review

62stars1forks

View on GitHub

How to install general-code-review?

lucasfcosta/backpressured/general-code-review

$npx -y skills add lucasfcosta/backpressured --skill general-code-review

Installs into the current project.

Summary

Use when a backpressured code-review subagent is reviewing a diff for correctness/logic, simplicity/reuse, or test-quality defects — everything except type design. The default reviewer dimension.

Files · 1

View on GitHub

SKILL.md

1	# General Code Review
2
3	## Overview
4
5	You are a machine the producer ran so a human wouldn't have to be the one to catch this. Find the defects, the needless complexity, and the missing test coverage that would otherwise survive until a human reviewer — or production. Report what is actually wrong with evidence, not stylistic opinions.
6
7	This is the catch-all reviewer covering three dimensions. Type-level concerns — illegal states being representable, casts, exhaustiveness — are not here; route those to [[type-design-review]].
8
9	## When to Use
10
11	- A `backpressured` code-review subagent is reviewing a diff (per-iteration or whole-changeset) and needs correctness, simplicity, and test-quality coverage.
12	- This is the default dimension — almost every diff warrants it. Run it alongside [[type-design-review]] (and any future reviewer skills) when those also apply.
13
14	Not for: type-design findings (use [[type-design-review]]); pure formatting a linter already enforces; bikeshedding naming with no correctness or clarity impact.
15
16	## Dimension 1 — Correctness & logic
17
18	Does it actually do the right thing, on the unhappy paths too?
19
20	- Edge & boundary cases: empty/single/huge inputs, zero, negative, off-by-one, first/last iteration, overflow.
21	- Error handling: are errors caught, propagated, or swallowed? Is a rejected promise / non-zero exit / partial failure handled? Are resources released on the error path (no leaked handles, locks, transactions)?
22	- Null/undefined/absent: is "missing" handled distinctly from "empty" or "zero"?
23	- Concurrency: shared mutable state, races, await-in-loop ordering, non-atomic read-modify-write, unguarded caches.
24	- Logic: inverted conditionals, wrong boolean operator, `==` vs `===`, mutating a collection while iterating, incorrect early return, assumptions that the data was already validated upstream (was it?).
25	- Contract drift: does the change match its callers, the docs, the tests, and the stated intent? Did a signature change leave a caller behind?
26
27	Hand-off: if the right fix is to encode an invariant in a type — parse instead of trusting "validated upstream", a branded id instead of a raw string, a union instead of a flag — that is a type-design finding; route it to [[type-design-review]] rather than fixing it as a runtime check here.
28
29	## Dimension 2 — Simplicity & reuse
30
31	Could this be smaller, clearer, or stop repeating itself? (Same lens as `/simplify`.)
32
33	- Reuse: is there an existing helper/util/type this duplicates? Prefer calling it over re-implementing.
34	- Duplication: the same logic in 2–3 places that should be one function.
35	- Dead code: unused vars, unreachable branches, commented-out blocks, params nobody passes.
36	- Over-engineering: abstraction with one caller, configurability nobody asked for, a class where a function would do, premature generality.
37	- Altitude: a function doing too many things at once; deeply nested conditionals that flatten with early returns; a long parameter list that wants an object.
38	- Naming & clarity: names that mislead or under-describe; a comment that exists only because the code is unclear (fix the code). Per house style: comments explain why, not what.
39
40	## Dimension 3 — Test quality
41
42	Tests are the machine that says "no" on the next change — judge whether they actually will.
43
44	- New behavior is covered: the diff's new logic and its edge cases have tests that would fail if the behavior regressed.
45	- Tests assert behavior, not implementation: they check outcomes/observable effects, not internal call sequences or private state. A test that breaks on a harmless refactor is testing the wrong thing.
46	- Over-mocking: mocking the very thing under test, or so much that the test only proves the mocks were called. Prefer testing through real collaborators; mock only at genuine boundaries (network, clock, fs).
47	- Top-down coverage: a higher-level test that exercises the lower levels too is better than many brittle isolated unit tests. Flag missing coverage of the integrated path; don't pile on isolated unit tests for trivial code a higher-level test already covers (this is about preferring the right level, not deleting fast unit tests that earn their place).
48	- Edge & negative cases: not just the happy path — error inputs, empty inputs, the boundary values from Dimension 1.
49	- Determinism: no reliance on real time, random, network, or ordering; no leaked state between tests.
50
51	## How to report findings
52
53	For each finding give: dimension (correctness / simplicity / test), severity ([BLOCKER] real defect or missing coverage of new behavior · [SHOULD] clear improvement · [NIT] minor/optional), location (`file:line` or symbol), and a

People who liked this also viewed

6 picks

Type