InternLM/xtuner

# XTuner Programming Standards

## Python

### Development Standards

1. When adding methods or functions to a module or class, declare **public interfaces** as early as possible and place **private interfaces** toward the end.
2. Python **>= 3.10** is required. Use modern syntax accordingly (e.g., `X | Y` union types).
3. Use `{}` instead of `dict()` for creating dictionaries. Use `[]` instead of `list()` for creating lists.
4. Use **double quotes** (`"`) for strings instead of single quotes (`'`).
5. Use `TypedDict` with `{}` syntax when a parameter requires a typed dictionary.
6. Prefer `pathlib.Path` over `os.path` for filesystem operations.
7. All new code must include **type hints** for function signatures (parameters and return types).
8. Maximum line length is **119** characters (configured in `pyproject.toml`).

### Docstring Standards

1. Use **Google Style** docstrings.
2. For classes, include `__init__` documentation directly under the **class docstring**; do not repeat it under the `__init__` method.
3. Parameter types **must** be declared in docstrings — include the type hint in parentheses immediately following the parameter name.
4. The **return type** must be explicitly declared in the docstring.
5. Do **not** declare raised exceptions in the docstring.
6. Do **not** declare Attributes in the class docstring.
7. If the return value is `None`, omit the `Returns` section.
8. Only provide docstrings for **public methods**. Private methods do not require docstrings.

### Linting, Formatting & Type Checking

- **Ruff** for linting and formatting, **mypy** for type checking — see `pyproject.toml` for configuration.
- `mypy` is strict for `xtuner/v1/*` (`ignore_missing_imports = false`).
- Pre-commit hooks apply **only** to `xtuner/v1/` and `autotest/` — run `pre-commit run --all-files` before submitting a PR.
- Docstrings are wrapped at **119** characters by `docformatter`.

### Error Handling

1. Never use bare `except:` — always catch specific exception types.
2. Avoid silencing exceptions with `pass` unless there is a documented reason.

### Testing

1. Every bug fix PR **must** include a regression test that reproduces the original bug.
2. Test functions must be named `test_<description>` and grouped in classes named `Test<Feature>`.
3. Use `pytest` as the test framework. Prefer `pytest.raises` for exception assertions.
4. Tests should be self-contained — avoid reliance on external services or GPUs unless explicitly marked with `@pytest.mark.gpu`.

---

## Git

### Branch Naming

- Bug fixes: `fix/<short-description>`
- Features: `feat/<short-description>`
- Refactors: `refactor/<short-description>`

### Commit Message Standards

1. Follow the style of previous commit messages to maintain a **consistent** project history.
2. Use the format: `[Tag] Short description` (e.g., `[Fix] Fix OOM in MoE expert routing`).
3. Common tags: `[Fix]`, `[Feature]`, `[Refactor]`, `[CI]`, `[Docs]`, `[Test]`.
4. **Never** include metadata or watermarks unrelated to the code itself, such as "generated by AI" or "Co-Authored-By: Claude."

---

## Pull Request Standards

### Bug Fix PRs

1. **Title**: `[Fix] <concise description of the bug>`
2. **Body** must include:
   - **Root Cause**: A clear explanation of why the bug occurred.
   - **Fix**: What was changed and why this is the correct fix.
   - **Test Plan**: How the fix was verified (unit test, manual reproduction, etc.).
3. Keep the diff **minimal** — fix only the bug. Do not refactor surrounding code, add unrelated improvements, or change formatting of untouched lines.
4. If the fix touches performance-critical paths (data loading, forward/backward pass, communication), include **before/after** benchmarks or justify why benchmarks are unnecessary.

### General PR Rules

1. One logical change per PR. Do not mix bug fixes with features or refactors.
2. All CI checks must pass before requesting review.
3. Resolve all review comments before merging — do not dismiss without explanation.

---

## Code Review Standards

### What to Check

1. **Correctness**: Does the code do what it claims? Are edge cases handled?
2. **Concurrency & Distributed**: Check for race conditions, deadlocks, incorrect `dist.barrier()` placement, and mismatched collective operations across ranks.
3. **Memory**: Watch for GPU memory leaks — tensors not freed, caches growing unbounded, missing `torch.no_grad()` in inference paths.
4. **Numerical Stability**: Flag potential issues like `log(0)`, division by zero, `float32`/`float16` overflow in loss computation or gradient scaling.
5. **Performance**: Unnecessary CPU-GPU synchronization (`item()`, `.cpu()` in hot paths), redundant data copies, inefficient collective communication patterns.
6. **API Contracts**: Do public interface changes maintain backward compatibility? Are deprecation warnings added for breaking changes?
7. **Resource Cleanup**: Are file handles, NCCL communicators, and CUDA streams properly cleaned up?

### Review Output Format

When performing code reviews, use the following structure:

```
## Summary
<1-2 sentence overview of the change>

## Issues
### Critical
- [file:line] Description of the issue and suggested fix

### Warning
- [file:line] Description of the concern

### Nit
- [file:line] Minor style or readability suggestion

## Verdict
APPROVE / REQUEST_CHANGES / COMMENT
```

### Review Principles

1. **Be specific** — always reference the exact file and line number.
2. **Explain why** — don't just say "this is wrong"; explain the failure mode.
3. **Suggest a fix** — provide a concrete code suggestion when possible.
4. **Distinguish severity** — separate critical bugs from style nits.
5. Do not flag issues in code that is **outside the scope** of the PR diff.



## Rules

Please refer to the docs in `./dev-rules` for the development guidelines.