From 0533502064571fb0bda70260579e567404258eae Mon Sep 17 00:00:00 2001 From: Pratik Patel Date: Wed, 4 Feb 2026 16:40:35 -0800 Subject: [PATCH 1/2] Add Claude AI coding guidelines - CLAUDE.md: Generic generation rules (style, architecture, testing, error handling, security) - CLAUDE-REVIEW.md: Code review checklist - CLAUDE-GO.md: Go-specific guidelines Co-Authored-By: Claude Opus 4.5 --- CLAUDE-GO.md | 324 +++++++++++++++++++++++++++++++++++++++++++++++ CLAUDE-REVIEW.md | 114 +++++++++++++++++ CLAUDE.md | 186 +++++++++++++++++++++++++++ 3 files changed, 624 insertions(+) create mode 100644 CLAUDE-GO.md create mode 100644 CLAUDE-REVIEW.md create mode 100644 CLAUDE.md diff --git a/CLAUDE-GO.md b/CLAUDE-GO.md new file mode 100644 index 00000000..37681c62 --- /dev/null +++ b/CLAUDE-GO.md @@ -0,0 +1,324 @@ +# Go-Specific Guidelines + +These rules supplement the generic guidelines in `CLAUDE.md`. Follow both. + +--- + +## Code Style + +### Formatting and linting + +Run `gofmt` and `golangci-lint` before committing. The linter config is authoritative—if it passes, the style is acceptable. + +### Variable naming + +Use short names in small scopes, descriptive names in larger scopes. + +```go +// In a 3-line function, this is fine: +for i, v := range items { ... } + +// In a 50-line function, be explicit: +for itemIndex, currentItem := range inventoryItems { ... } +``` + +### Receiver names + +Use short, consistent receiver names. Never use `this` or `self`. + +```go +// Do: +func (s *Service) Create(ctx context.Context) error { ... } + +// Don't: +func (this *Service) Create(ctx context.Context) error { ... } +func (service *Service) Create(ctx context.Context) error { ... } +``` + +--- + +## Architecture + +### Package organization + +- Package by feature/domain, not by layer +- Use `internal/` for packages that shouldn't be imported externally +- Keep `cmd/` minimal—wire things up and call `Run()` + +``` +internal/ + user/ # User domain (service, repo, handlers) + billing/ # Billing domain + notification/ # Notification domain +pkg/ # Reusable utilities +cmd/ # Entry points only +``` + +### Interface placement + +Interfaces belong with the consumer, not the implementer. Define interfaces where you need them. + +```go +// In the consumer package: +type UserRepository interface { + Get(ctx context.Context, id string) (*User, error) +} + +// Not in the repository package alongside the implementation +``` + +### Dependency injection + +Pass dependencies explicitly. Prefer constructor injection over global state. + +```go +// Do: +func NewService(repo Repository, logger *zap.Logger) *Service { + return &Service{repo: repo, logger: logger} +} + +// Don't: +var globalRepo Repository // package-level state +``` + +--- + +## Error Handling + +### Always wrap errors with context + +Use `fmt.Errorf` with `%w` or your error package's wrap function. Include what operation failed. + +```go +// Do: +if err != nil { + return fmt.Errorf("failed to create user %s: %w", userID, err) +} + +// Don't: +if err != nil { + return err +} +``` + +### Check errors immediately + +Handle errors right after the call that might produce them. Don't defer error checking. + +```go +// Do: +result, err := doSomething() +if err != nil { + return err +} +// use result + +// Don't: +result, err := doSomething() +// ... other code ... +if err != nil { // easy to forget or misplace + return err +} +``` + +### Use sentinel errors for expected cases + +When callers need to handle specific error types, use sentinel errors or typed errors. + +```go +var ErrNotFound = errors.New("not found") +var ErrConflict = errors.New("conflict") + +// Callers can check: +if errors.Is(err, ErrNotFound) { + // handle not found +} +``` + +### Don't panic for normal errors + +Reserve `panic` for truly unrecoverable situations (programmer errors, invariant violations). Normal errors should be returned. + +```go +// Do: +func ParseConfig(path string) (*Config, error) { + // return error if file doesn't exist +} + +// Don't: +func MustParseConfig(path string) *Config { + // panic if file doesn't exist +} +``` + +--- + +## Testing + +### Table-driven tests + +Use table-driven tests when testing multiple cases of the same behavior. + +```go +func TestValidateEmail(t *testing.T) { + tests := []struct { + name string + email string + wantErr bool + }{ + {"valid email", "user@example.com", false}, + {"missing @", "userexample.com", true}, + {"empty", "", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateEmail(tt.email) + if (err != nil) != tt.wantErr { + t.Errorf("ValidateEmail(%q) error = %v, wantErr %v", tt.email, err, tt.wantErr) + } + }) + } +} +``` + +### Test naming convention + +Use descriptive names that explain the scenario: + +```go +func Test_CreateUser_WithDuplicateEmail_ReturnsConflictError(t *testing.T) +func Test_GetUser_WhenNotFound_ReturnsNil(t *testing.T) +``` + +### Use t.Helper() in test helpers + +Mark helper functions so test failures report the correct line. + +```go +func assertNoError(t *testing.T, err error) { + t.Helper() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} +``` + +### Be consistent with assertion libraries + +Pick one (testify/assert, stdlib, etc.) and stick with it across the codebase. + +--- + +## Concurrency + +### Context cancellation + +Always respect context cancellation. Check `ctx.Done()` in long-running operations. + +```go +func ProcessItems(ctx context.Context, items []Item) error { + for _, item := range items { + select { + case <-ctx.Done(): + return ctx.Err() + default: + if err := process(item); err != nil { + return err + } + } + } + return nil +} +``` + +### Use errgroup for parallel operations + +When running multiple operations in parallel that can fail, use `errgroup`. + +```go +g, ctx := errgroup.WithContext(ctx) + +for _, item := range items { + item := item // capture loop variable + g.Go(func() error { + return processItem(ctx, item) + }) +} + +if err := g.Wait(); err != nil { + return err +} +``` + +### Document goroutine ownership + +When spawning goroutines, document who owns them and how they're cleaned up. + +```go +// Start starts the background processor. +// The goroutine runs until ctx is cancelled. +// Caller must call Stop() or cancel the context to clean up. +func (p *Processor) Start(ctx context.Context) { + go p.run(ctx) +} +``` + +### Prefer channels for communication, mutexes for state + +Use channels to coordinate between goroutines. Use mutexes to protect shared state. + +```go +// Channel for work coordination: +jobs := make(chan Job) + +// Mutex for shared state: +var mu sync.Mutex +var count int +``` + +--- + +## Observability + +### Prefer spans over log lines + +Use tracing spans instead of log lines for observability, especially at info level. Spans provide context, timing, and can be correlated across services. Reserve logging for errors (which should still include useful details). + +```go +// Do: Start a span for the operation +ctx, span := tracer.Start(ctx, "CreateUser") +defer span.End() + +span.SetAttributes(attribute.String("user.email", email)) + +// Don't: Log every operation +log.Info("creating user", "email", email) +log.Info("user created", "id", userID) +``` + +### Ensure methods have spans for tracing + +Important methods—especially service boundaries, database operations, and external API calls—should have spans so traces are useful for debugging. + +```go +func (s *Service) CreateUser(ctx context.Context, req *CreateUserRequest) (*User, error) { + ctx, span := tracer.Start(ctx, "UserService.CreateUser") + defer span.End() + + // ... implementation +} +``` + +### Errors can be logged in addition to spans + +When an error occurs, it's fine to log it with context in addition to recording it on the span. Include useful details like identifiers and operation context. + +```go +if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, "failed to create user") + log.Error("failed to create user", "email", email, "error", err) + return nil, err +} +``` diff --git a/CLAUDE-REVIEW.md b/CLAUDE-REVIEW.md new file mode 100644 index 00000000..7aec557f --- /dev/null +++ b/CLAUDE-REVIEW.md @@ -0,0 +1,114 @@ +# Code Review Guidelines + +Use this checklist when reviewing pull requests. These guidelines apply regardless of whether the code was written by a human or AI. + +For language-specific review points, see `CLAUDE-GO.md` or `CLAUDE-TS.md`. + +--- + +## Completeness + +- [ ] **Does the PR actually solve what it claims?** Read the PR description and linked issue, then verify the code addresses it +- [ ] **Are edge cases handled?** Empty inputs, null values, boundary conditions, concurrent access +- [ ] **Are error paths covered?** What happens when things fail? Network errors, invalid data, missing resources +- [ ] **Is the acceptance criteria met?** If there's a spec or requirements, check each item + +### Red flags +- PR title says "Add feature X" but feature X is only partially implemented +- Happy path works but error cases return generic errors or crash +- No consideration of what happens with empty/null/zero inputs + +--- + +## Regression Risk + +- [ ] **Could this break existing functionality?** Look for changes to shared code, interfaces, or data structures +- [ ] **Are there integration points that might be affected?** APIs, database schemas, message formats +- [ ] **Have dependent systems been considered?** Other services that call this code, downstream consumers +- [ ] **Are there breaking changes?** API contract changes, removed fields, changed behavior + +### Red flags +- Changes to widely-used utility functions without updating all callers +- Database migrations that could break existing queries +- Renamed or removed API fields without versioning + +--- + +## Test Coverage + +- [ ] **Are there tests for the new/changed behavior?** Not just "tests exist" but "tests cover this change" +- [ ] **Do tests actually validate the change?** Tests should fail if the feature breaks, not just execute the code +- [ ] **Are failure scenarios tested?** Error handling, edge cases, invalid inputs +- [ ] **Is the test plan clear?** Either documented or obvious from test names + +### Red flags +- Tests that only check the happy path +- Tests that mock so much they don't test real behavior +- "Tested manually" without explanation of what was tested +- Tests that would pass even if the feature was completely broken + +--- + +## Scope + +- [ ] **Is the PR focused on a single concern?** One feature, one bug fix, or one refactor—not all three +- [ ] **Should this be split?** Large PRs are harder to review and riskier to merge +- [ ] **Are there unrelated changes?** Drive-by refactors, formatting changes, unrelated fixes +- [ ] **Is the PR a reasonable size?** Rule of thumb: if it takes more than 30 minutes to review, it's probably too big + +### Red flags +- PR description lists multiple unrelated items +- Changes span many unrelated files +- "While I was in there, I also..." changes +- 1000+ line PRs (unless it's mostly generated code or tests) + +--- + +## Consistency + +- [ ] **Does this match existing patterns?** Look at how similar things are done elsewhere in the codebase +- [ ] **Are naming conventions followed?** Variables, functions, files, packages +- [ ] **Is the code style consistent?** With surrounding code and project standards +- [ ] **Are established libraries used?** Don't reinvent what already exists in the codebase + +### Red flags +- New patterns introduced when existing ones would work +- Different naming style than the rest of the codebase +- Custom implementation of something that exists in a shared utility +- Third-party library added when a standard library solution exists + +--- + +## Reviewability + +- [ ] **Is the PR description clear?** Explains what changed and why, not just "fixes bug" +- [ ] **Are commits logically organized?** Each commit should be a coherent unit (though not required) +- [ ] **Is complex logic explained?** Either in code comments or PR description +- [ ] **Can you understand it without asking?** If you need a walkthrough, the code may be too complex + +### Red flags +- Empty or minimal PR description +- "WIP" or "misc fixes" without details +- Complex algorithms with no explanation +- Magic numbers or obscure logic without comments + +--- + +## How to Give Feedback + +### Be specific +``` +// Don't: "This could be improved" +// Do: "This query runs N+1 times in a loop. Consider batching the lookups." +``` + +### Explain why +``` +// Don't: "Use X instead of Y" +// Do: "Use X instead of Y because X handles the edge case where Z" +``` + +### Distinguish severity +- **Blocking**: Must fix before merge (bugs, security issues, breaking changes) +- **Should fix**: Important but could be a follow-up (test coverage gaps, minor issues) +- **Nit**: Style preferences, suggestions, take-it-or-leave-it diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..a242fa14 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,186 @@ +# Code Generation Guidelines + +These guidelines apply to all code generation, regardless of language. For language-specific rules, see `CLAUDE-GO.md` or `CLAUDE-TS.md`. + +--- + +## Code Style + +### Follow idiomatic patterns + +Write code that looks like it belongs in the language you're using. Follow the conventions, idioms, and best practices of that language and its ecosystem. When in doubt, look at how the standard library or well-regarded projects do it. + +### Functions should tend to do one thing + +Functions should have a reasonable scope with a clear purpose. If you find yourself using "and" to describe what a function does, consider splitting it. Keep functions short enough to understand at a glance—if it doesn't fit on one screen, it's probably doing too much. + +``` +// Don't: validateAndSaveUser() +// Do: validate(), then save() separately +``` + +### Prefer explicit over clever, but stay concise + +Code is read more often than it's written. Prioritize clarity over brevity, but also avoid unnecessary verbosity—too much code is just as hard to read as too little. Find the balance where the code is immediately understandable without being bloated. + +``` +// Don't: obscure one-liners or overly verbose boilerplate +// Do: clear, straightforward code that says what it means +``` + +### Name things for what they represent + +Names should describe the thing itself, not how it's used or where it came from. + +``` +// Don't: tempList, dataFromAPI, processedResult +// Do: users, orders, validatedItems +``` + +### Avoid code duplication + +Don't copy-paste code. Extract shared logic into reusable functions, utilities, or base classes. Prefer extension and composition over duplication. When you see repeated patterns, consolidate them. + +### Be mindful of file organization + +Trend away from multi-thousand line files. If a file is growing large, look for natural boundaries to split it. Related code should be grouped together, but massive files become hard to navigate and maintain. + +### Be wary of accidental mutation + +Understand whether your language passes by value or reference. When working with objects or collections, be explicit about whether you're modifying the original or creating a copy. Accidental mutation causes subtle bugs. + +--- + +## Architecture + +### Clear separation of concerns + +Each module/package should have a clear, singular purpose. Business logic shouldn't know about HTTP. Database access shouldn't know about presentation. + +### Dependencies flow inward + +External layers (HTTP handlers, CLI) depend on internal layers (business logic), which depend on core domain. Never the reverse. + +``` +handlers → services → domain + ↓ ↓ + repos (no outward deps) +``` + +### Interfaces at boundaries + +Define interfaces where modules meet. This allows swapping implementations (real DB vs mock) and makes testing easier. + +### Avoid circular dependencies + +If A imports B and B imports A, extract shared code to a new package C that both can import. + +### Consider structural impact + +When making changes, consider how they affect the overall codebase structure. If a change is making the code messier or harder to navigate, surface this to the user. Suggest refactoring instead of polluting the existing structure with workarounds. + +--- + +## Testing + +### Every feature needs a test plan + +Before writing code, know how you'll verify it works. This could be unit tests, integration tests, or manual verification steps—but decide upfront. + +### Test behavior, not implementation + +Tests should verify what the code does, not how it does it. If you refactor internals without changing behavior, tests shouldn't break. + +``` +// Don't: assert that a specific internal method was called 3 times +// Do: assert that the output is correct for given inputs +``` + +### Use descriptive test names + +A test name should explain the scenario being tested. When a test fails, the name should tell you what broke. + +``` +// Don't: TestUser, TestValidation +// Do: TestCreateUser_WithDuplicateEmail_ReturnsConflictError +``` + +### Prefer integration tests for I/O-heavy code + +For code that primarily shuffles data between systems (APIs, databases), integration tests provide more confidence than unit tests with mocks. + +--- + +## Error Handling + +### Errors should provide context + +When an error occurs, include what operation failed and relevant identifiers. Raw errors like "connection refused" don't tell you which service or what operation. + +``` +// Don't: return err +// Do: return error wrapping with "failed to fetch user {id}: {err}" +``` + +### Don't swallow errors silently + +Every error should be either: +1. Handled and recovered from +2. Logged with context +3. Returned/propagated to the caller + +Never ignore an error without explicit justification. + +### Handle errors at the appropriate level + +Don't handle errors too early (before you have context to handle them properly) or too late (after the context is lost). Handle where you can actually do something meaningful. + +### Distinguish recoverable vs unrecoverable errors + +Some errors are expected (user not found, validation failed) and should be handled gracefully. Others indicate bugs or system failures and should fail loudly. + +### Be mindful of log levels and spam + +Use appropriate log levels: errors for actual problems, info for significant events, debug for detailed diagnostics. Avoid logging the same information repeatedly in hot paths. Include useful details, but don't spam logs with noise that makes it hard to find what matters. + +--- + +## Security + +### Validate all external input + +Anything from outside the system boundary (user input, API responses, file contents) must be validated before use. Trust nothing external. + +### Never log sensitive data + +Secrets, tokens, passwords, and PII should never appear in logs. When in doubt, don't log it. Mask or redact if you must reference it. + +``` +// Don't: log.Info("authenticating user", "token", token) +// Do: log.Info("authenticating user", "user_id", userID) +``` + +### Use parameterized queries + +Never concatenate user input into queries. Always use parameterized queries or prepared statements to prevent injection attacks. + +``` +// Don't: query := "SELECT * FROM users WHERE id = " + userID +// Do: query with parameterized placeholder and userID as parameter +``` + +### Principle of least privilege + +Request only the permissions you need. Don't use admin credentials when read-only access suffices. Scope access tokens narrowly. + +--- + +## Working on Large Changes + +### Split large changes into reviewable chunks + +If a change is approaching ~1000 lines and there's more to do, ask about splitting the work. A good pattern: +1. First PR: structural changes (interfaces, method stubs, abstractions) +2. Second PR: fill in the implementation + +This keeps each PR human-reviewable and logically organized. Large monolithic changes are hard to review and risky to merge. From c5705d30eb2d150ba8daefa2d0adb1052da24a3e Mon Sep 17 00:00:00 2001 From: Pratik Patel Date: Wed, 4 Feb 2026 16:52:16 -0800 Subject: [PATCH 2/2] Add reference to companion guideline files in CLAUDE.md Co-Authored-By: Claude Opus 4.5 --- CLAUDE.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index a242fa14..110a1705 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,5 +1,7 @@ # Code Generation Guidelines +> **Also read**: `CLAUDE-GO.md` and `CLAUDE-REVIEW.md` and `CLAUDE-LOCAL.md` (if present) for complete guidance. + These guidelines apply to all code generation, regardless of language. For language-specific rules, see `CLAUDE-GO.md` or `CLAUDE-TS.md`. ---