TypeScript Code Review
Overview
Score every code review from 100 → 0. Start at 100 and apply deductions.
Issues detectable by ESLint, Prettier, or tsc --strict are NOT scored here.
Scoring Summary
| Category | Deduction | Per |
|---|---|---|
| Bidirectional dependency | -20 | per occurrence |
| Duplicated logic | -10 | per occurrence |
| Violates project design (AGENTS.md) | -10 | per occurrence |
| Non-refactoring-tolerant test | -3 | per test case |
| WhiteBox test | -3 | per test case |
| Test file not using separate test scope | -3 | per file |
| Uncovered code | -1 | per line |
| Security HIGH | -10 | per finding |
| Security MEDIUM | -5 | per finding |
| Security LOW | -1 | per finding |
| Spec not met | -100 | — |
| TypeScript best practice violation | varies | per occurrence |
Architecture
Bidirectional Dependency (-20 per occurrence)
Modules must depend in one direction only. Types-only modules (e.g. types.ts) are allowed as shared dependencies.
typescript
// ❌ BAD: bidirectional dependency
// order.ts imports payment
import { Processor } from './payment';
export function createOrder(p: Processor) { ... }
// payment.ts imports order
import { Order } from './order';
export function charge(o: Order) { ... }
typescript
// ✅ GOOD: unidirectional with shared types
// types.ts — no imports from order or payment
export interface Order {
id: string;
amount: number;
}
// payment.ts depends on types only
import { Order } from './types';
export function charge(o: Order): void { ... }
// order.ts depends on types and payment
import { Order } from './types';
import { charge } from './payment';
export function place(o: Order): void {
charge(o);
}
Duplicated Logic (-10 per occurrence)
Extract shared behavior into a function or module.
typescript
// ❌ BAD: duplicated validation
function createUser(name: string): void {
if (!name || name.length > 100) {
throw new Error('invalid name');
}
// ...
}
function updateUser(name: string): void {
if (!name || name.length > 100) {
throw new Error('invalid name');
}
// ...
}
typescript
// ✅ GOOD: shared validation
function validateName(name: string): void {
if (!name || name.length > 100) {
throw new Error('invalid name');
}
}
function createUser(name: string): void {
validateName(name);
// ...
}
function updateUser(name: string): void {
validateName(name);
// ...
}
Project Design Violation (-10 per occurrence)
Code must follow the architecture described in the project's AGENTS.md. Check layer boundaries, naming conventions, and module responsibilities.
Unit Testing
Non-Refactoring-Tolerant Test (-3 per test case)
Tests must assert results, not how the code works internally. Asserting call arguments, call counts, or internal state couples tests to implementation.
typescript
// ❌ BAD: asserts mock call arguments
test('transfer', () => {
const mockRepo = { debit: jest.fn(), credit: jest.fn() };
const svc = new Service(mockRepo);
svc.transfer('alice', 'bob', 100);
// Asserting internal call args = breaks on refactor
expect(mockRepo.debit).toHaveBeenCalledWith('alice', 100);
expect(mockRepo.credit).toHaveBeenCalledWith('bob', 100);
});
typescript
// ✅ GOOD: asserts observable outcome
test('transfer', () => {
const repo = new InMemoryRepo();
repo.setBalance('alice', 200);
repo.setBalance('bob', 50);
const svc = new Service(repo);
svc.transfer('alice', 'bob', 100);
expect(repo.getBalance('alice')).toBe(100);
expect(repo.getBalance('bob')).toBe(150);
});
WhiteBox Test (-3 per test case)
Always use BlackBox testing. Tests must exercise the public API from an external perspective.
typescript
// ❌ BAD: importing and testing internal function
import { _internalCalculate } from './wallet';
test('internal calculation', () => {
expect(_internalCalculate(10, 20)).toBe(30);
});
typescript
// ✅ GOOD: testing only the public API
import { Wallet } from './wallet';
test('deposit increases balance', () => {
const w = new Wallet();
w.deposit(100);
expect(w.balance()).toBe(100);
});
Test Scope (-3 per file)
Tests should only import public API. Never export internals for testing purposes.
typescript
// ❌ BAD: exporting internal for test access
// wallet.ts
export const _internal = { calculate }; // test-only export
// ✅ GOOD: test only through public interface
// wallet.ts exports only Wallet class
export class Wallet { ... }
Uncovered Code (-1 per line)
Every line of production code should be exercised by tests. Run:
bash
npx vitest --coverage
Security
Review security against the project context (README.md / AGENTS.md).
| Severity | Deduction | Example |
|---|---|---|
| HIGH (-10) | SQL injection, hardcoded secrets, missing auth | Template literal in SQL, const API_KEY = "sk-..." |
| MEDIUM (-5) | Insufficient input validation, weak crypto | Using md5 for password hashing |
| LOW (-1) | Missing rate limiting, verbose error messages | Returning stack traces to client |
typescript
// ❌ HIGH: SQL injection
async function getUser(id: string): Promise<User> {
return db.query(`SELECT * FROM users WHERE id = '${id}'`);
}
typescript
// ✅ GOOD: parameterized query
async function getUser(id: string): Promise<User> {
return db.query('SELECT * FROM users WHERE id = $1', [id]);
}
typescript
// ❌ HIGH: hardcoded secret
const stripe = new Stripe('sk_live_abc123xyz');
typescript
// ✅ GOOD: secret from environment const stripe = new Stripe(process.env.STRIPE_SECRET_KEY!);
Specification
Spec Not Met (-100)
If the code does not satisfy the specification, the review score is effectively 0. Verify behavior against the requirements before anything else.
TypeScript Best Practices
Using any (-5 per occurrence)
typescript
// ❌ BAD: defeats type safety
function process(data: any): any {
return data.value;
}
typescript
// ✅ GOOD: proper typing
interface Payload {
value: string;
}
function process(data: Payload): string {
return data.value;
}
Non-Null Assertion Abuse (-3 per occurrence)
typescript
// ❌ BAD: hiding potential null
function getName(user?: User): string {
return user!.name;
}
typescript
// ✅ GOOD: explicit null handling
function getName(user?: User): string {
if (!user) {
throw new Error('user is required');
}
return user.name;
}
Untyped Error Handling (-3 per occurrence)
typescript
// ❌ BAD: untyped catch
try {
await fetchData();
} catch (e) {
console.log(e.message); // e is unknown
}
typescript
// ✅ GOOD: typed error handling
try {
await fetchData();
} catch (e) {
if (e instanceof AppError) {
console.log(e.message);
} else {
throw e;
}
}
Mutable Shared State (-5 per occurrence)
typescript
// ❌ BAD: global mutable state
let cache: Record<string, User> = {};
export function getUser(id: string): User {
return cache[id];
}
export function setUser(id: string, user: User): void {
cache[id] = user;
}
typescript
// ✅ GOOD: encapsulated state
export class UserCache {
private readonly cache = new Map<string, User>();
get(id: string): User | undefined {
return this.cache.get(id);
}
set(id: string, user: User): void {
this.cache.set(id, user);
}
}
Promise / Async Misuse (-3 per occurrence)
typescript
// ❌ BAD: async function that doesn't need to be
async function add(a: number, b: number): Promise<number> {
return a + b;
}
// ❌ BAD: missing await
async function save(user: User): Promise<void> {
db.insert(user); // forgot await — fire and forget
}
typescript
// ✅ GOOD: sync when no async needed
function add(a: number, b: number): number {
return a + b;
}
// ✅ GOOD: awaiting async operations
async function save(user: User): Promise<void> {
await db.insert(user);
}
Stringly-Typed API (-3 per occurrence)
typescript
// ❌ BAD: status as string
function setStatus(order: Order, status: string): void {
order.status = status;
}
typescript
// ✅ GOOD: union type for known variants
type OrderStatus = 'pending' | 'shipped' | 'delivered';
function setStatus(order: Order, status: OrderStatus): void {
order.status = status;
}
Barrel Export Bloat (-2 per occurrence)
typescript
// ❌ BAD: re-exporting everything from index.ts // index.ts export * from './user'; export * from './order'; export * from './payment'; export * from './internal-helpers'; // leaking internals
typescript
// ✅ GOOD: explicit public API
// index.ts
export { createUser, getUser } from './user';
export { placeOrder } from './order';
export { charge } from './payment';
Review Procedure
- •Spec — Does the code meet the requirements? If not, stop (-100).
- •Architecture — Check dependency direction, duplication, project design.
- •Unit Tests — BlackBox? Refactoring-tolerant? No internal exports? Coverage?
- •Security — Review against project context for vulnerabilities.
- •TypeScript Best Practices — Types, error handling, async, state management.
- •Calculate Score — Start at 100, apply all deductions. Minimum is 0.
Output Format
code
## Code Review: [component/file] **Score: XX/100** ### Deductions | # | Category | Detail | Deduction | |---|----------|--------|-----------| | 1 | Architecture | Bidirectional dep: order ↔ payment | -20 | | 2 | Unit Test | WhiteBox test: internal calculation | -3 | | ... | ... | ... | ... | **Total Deductions: -XX** ### Summary [brief summary of key issues and recommendations]