[Template] Code Review Style Guide

[zhaochenyang20]

Code Style Guide

P0 — Correctness

Fail Fast

• Assert Invariants: Use assert for conditions that indicate programmer error. Use raise ValueError/TypeError for bad user input.
• Validate at Boundaries, Trust Internally: Validate inputs at public API entry points and system boundaries (HTTP handlers, config parsing). Internal helper functions can assume valid inputs.
• Early Returns: Guard clauses at the top of a function are preferred over deeply nested if-else blocks.
• Do Not Overprotect: LLMs tend to protect against extreme edge cases. If you believe an operation is correct 99% of the time, do not add defensive handling for its failure. Example
• Do Not Over-Catch: try/except should have a clear, narrow protection region. Do not wrap a large code block in a single try/except — it hides the real source of errors and makes debugging painful. Example

Concurrency & Thread Safety

• Document Thread Safety: If a class or function is accessed from multiple threads, document its thread-safety guarantees explicitly.
• Minimize Lock Scope: Hold locks for the shortest duration possible. Never perform I/O or GPU operations while holding a lock.
• Prefer Message Passing: When possible, use queues over shared mutable state.

Resource Management

• Explicit Tensor Lifecycle: Free large intermediate tensors with del + torch.cuda.empty_cache() only when profiling proves it necessary. Don't scatter these defensively.
• No Silent Resource Leaks: File handles, sockets, and CUDA streams must use context managers (with statements).
• Avoid Unbounded Growth: Any cache or buffer must have a bounded size or an eviction policy. No append-only lists in long-running loops.

P1 — Performance

Performance First

We are working on a high-performance system where every millisecond matters.

• Minimize Synchronization: Strictly avoid frequent calls to .item(), .cpu(), or .tolist() within the model inference path.
• Data Processing Vectorized: Keep data processing vectorized on the GPU whenever possible.
• Optimize Hot Paths: Prioritize low-overhead implementations for critical function paths. Example

P2 — Maintainability

Architecture & Decoupling

Maintain a clean codebase to facilitate collaboration and long-term maintenance. For features not yet widely accepted by the community, prioritize adding abstract base classes to /miles and implementing specific logic under /examples. Example Example 2

• Don't Repeat Yourself (DRY): Duplicate code snippets exceeding 5 lines must be extracted into shared functions.
• Keep Files Concise: If a single file exceeds 2,000 lines, it must be split into multiple smaller files (e.g., using Mixin patterns or sub-modules).
• Keep Functions Short: A single function should rarely exceed 50 lines (excluding docstrings). If it does, extract logical sub-steps into private helpers.

Naming Clarity

• No Abbreviations in Public APIs: Use request_count not req_cnt. Abbreviations are allowed only for widely-known terms (num, idx, cfg, bs for batch size).
• Boolean Names: Prefix with is_, has_, should_, can_ (e.g., is_ready, has_finished).
• Symmetry: Use consistent pairs — start/stop, begin/end, open/close, send/recv. Don't mix start/finish.
• No Generic Names: Avoid data, result, info, tmp, manager, handler without qualification. Use token_ids, decode_result, etc.

Imports

• Order: stdlib → third-party → local, separated by blank lines. Use isort to enforce.
• No Wildcard Imports: Never use from module import *.
• Lazy Imports for Heavy Dependencies: Import heavy packages (torch, triton) at module level, but optional or rare dependencies inside the function that uses them.
• No Circular Imports: If two modules need each other, extract the shared interface into a third module.

Constants

• No Magic Numbers: Extract numeric literals into named constants with descriptive names. MAX_BATCH_SIZE = 256 not just 256 inline.
• Exception: 0, 1, -1 in obvious arithmetic or indexing contexts are fine.

P3 — Style

Function Purity

Prioritize writing Pure Functions. Avoid in-place modification of input arguments.

Exception: If an in-place operation is required for extreme memory optimization (e.g., in the forward pass), it must be accompanied by an explicit comment.

Pythonic & Clean

• Lean Constructors: Keep __init__ parameters concise. Avoid passing massive, complex configuration objects; instead, pass only the necessary parameters.
• Avoid Dynamic Attributes: Minimize the use of getattr or setattr. Code should be explicit for better traceability. Example Example 2
• Ternary Operator Limits: Use a if condition else b only for very simple and clear cases. Complex logic must use standard if-else blocks.
• Extract Complex Logic: In multi-branch conditionals, if a specific branch contains multiple lines of logic, encapsulate it into a standalone private function.
• Complete Branching: When an if assigns a variable or returns a value, always include an else so every path is explicit. For guard clauses (early return/raise/continue), an else is not needed. If branching exceeds 3 levels, refactor into if/elif/.../else or a dispatch dict. Example Example 2
• Type Hints: All public APIs and function signatures must include type hints.
• Access Permission Indicator: Use _private style to indicate this is a function of this class or file only; otherwise, it's a public function that can be exposed outside.
• Clean Up the PR: Remove arbitrary comments and logs for debugging. Remove Chinese comments.

P4 — Process

Testing Efficiency

PR merge speed depends on verification/testing speed.

• Verification Script: Provide a verification script in your PR description that a reviewer can copy & paste to run immediately. Example
• CI Unit Tests: For important features (e.g., LoRA, SFT), add a CI unit test for the specific feature.
• Test the Contract, Not the Implementation: Tests should verify inputs → outputs, not internal state. This allows refactoring without rewriting tests.
• Deterministic Tests: Pin random seeds in tests. Flaky tests erode trust and slow CI.
• One Assertion Per Concept: Each test function should verify one logical behavior, even if it uses multiple assert statements to do so.