Review

/review - Code Review & Impact Audit

Review code changes against GitLens coding standards and verify change completeness.

Usage

/review [target]

• No argument: review staged changes (git diff --cached)
• all: all uncommitted changes
• file:path: specific file
• pr: current PR changes (gh pr diff)
• impact: impact completeness audit only (skip code review checklist)
• full: both code review + impact audit

Review Priorities

Review all changes for: correctness, matching user expectations, high performance (including proper caching and deferring of work), well-factored/structured/named code, minimal complexity, proper error handling and logging, comprehensive telemetry, and adherence to best practices. For webview changes, verify they are responsive, accessible, and work with VS Code theming.

Known Non-Issues (Do Not Flag)

• debugger; statements — Webpack's production build automatically strips all debugger statements via the minimizer. They are intentional development aids and NOT a shipping concern. Do NOT flag them in reviews.

Part 1: Code Review Checklist

TypeScript & Imports

• No any usage (exceptions only for external APIs)
• Explicit return types for public methods
• .js extension in all imports (ESM requirement)
• Import order: node built-ins → external → internal → relative
• import type for type-only imports
• No default exports
• Path aliases (@env/) for environment-specific code

Naming

• Classes: PascalCase (no I prefix)
• Methods/Variables: camelCase
• Constants: camelCase (not SCREAMING_SNAKE_CASE)
• Files: camelCase.ts
• Private members: leading underscore allowed

Error Handling

• Git errors use ErrorClass.is(ex, 'reason') pattern — not instanceof + .message.includes()
• Errors logged with context via Logger.error()
• No suppressed/ignored errors
• Graceful degradation for network/API failures

Localization

• Every new user-facing string is a literal l10n.t() message with placeholders for data — no concatenation, computed keys, or English plural suffixes
• Error text shown to a user comes from getPresentableErrorMessage(ex), not ex.message
• Git syntax, refs, paths, command ids and product names are placeholders, not translated text
• l10n/bundle.l10n.json regenerated when messages changed (pnpm run generate:l10n)

Performance

• Appropriate caching (@memoize(), GitCache, PromiseCache)
• Debounce expensive operations
• Lazy loading for heavy services

Webview (if applicable)

• Lit Elements for reactive UI
• VS Code theming via CSS custom properties (--vscode-*)
• Keyboard navigation and ARIA attributes
• disconnectedCallback() cleanup for listeners
• Z-index stays sane — a raw z-index climbing past ~100 is the signal to use a --gl-z-* tier token, isolation: isolate, or the top layer (<dialog>.showModal() / [popover]), not a bigger number (shadow-isolated internals keep small raw ordinals)

Scope & Simplicity

• No unnecessary new types/abstractions for single-use scenarios
• Changes scoped to the request — no unrelated drive-by changes
• Fix addresses root cause (feature not disabled instead of fixed)

Telemetry & Security

• Appropriate telemetry events for user actions
• No sensitive data in logs/telemetry
• No command injection, XSS, or hardcoded secrets

Documentation & Skills

• Check if changes affect patterns documented in AGENTS.md, docs/architecture.md, or docs/coding-standards.md — update if so
• Check if any skills in .claude/skills/ reference changed APIs, patterns, or file paths — update if so
• CHANGELOG entry needed for user-facing changes

Part 2: Impact Completeness Audit

Run automatically for changes spanning 3+ files, or when impact / full is specified.

1. Identify Changed Symbols

From the diff, extract:

• Modified function signatures and type definitions
• Renamed or removed exports
• Changed error handling or decorator usage

2. Find All Consumers

For each modified symbol:

• Search all import statements referencing the modified file
• Search all call sites of modified functions
• Search all implementations/overrides in subclasses
• Check per-operation git providers:
  • packages/git/src/providers/ (shared domain providers)
  • packages/git-cli/src/providers/ (CLI implementations)
  • src/plus/integrations/host/providers/ (host integrations, e.g. githubGitProvider.ts)

3. Platform Coverage

• Node.js code path (src/env/node/)
• Browser code path (src/env/browser/)
• Shared code (src/git/, src/system/, packages/)

4. Error & UI Impact

• New error cases handled at all catch sites
• Error types use .is() pattern correctly
• Webview IPC protocol changes reflected in both host and app
• Command changes reflected in contributions.json

Instructions

Code Review Flow

1. Get diff based on target
2. Read full files for context around each change
3. Analyze against Part 1 checklist
4. Categorize: Critical (must fix), Warnings (should fix), Suggestions (nice to have)
5. Include positive feedback for good patterns

Impact Audit Flow

1. Extract changed symbols from diff
2. Find all consumers of each symbol
3. Check platform and UI coverage
4. Report results:

### Consumers Checked

| Modified Symbol | Call Sites | All Updated?                    |
| --------------- | ---------- | ------------------------------- |
| functionA()     | 12         | Yes                             |
| TypeB           | 8          | 7/8 — MISSING: src/views/xyz.ts |

### Verdict

[COMPLETE / INCOMPLETE with remaining work]

Verification

pnpm exec tsc --noEmit    # Type-check
pnpm run lint              # Lint check