Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 47 additions & 6 deletions .github/workflows/claude-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,16 @@ jobs:
- ambiguous public signatures: boolean-trap parameters, multiple
same-typed positional runtime arguments that should be keyword-only,
or long anonymous tuple returns that need a named typed result.
- Model-specific ownership is mandatory. Model-specific logic may live
only under `nemo_automodel/components/models/<model>/`. Treat logic as
model-specific when it names or branches on a model or family, hard-codes
its module/parameter paths or weight layout, or implements its
architecture, initialization, or parallelization policy. If changed code
adds or expands such logic anywhere else, including another directory
under `nemo_automodel/components/` or a recipe, report a critical finding
and require relocation to the owning model package. Shared components may
expose generic typed contracts and invoke model-owned hooks, but must not
encode model identities or model-specific policy.
- Tensor contract docstrings. For every new function or method, or any
function/method whose signature or tensor-handling body is materially
changed, that accepts a tensor input, require its docstring to document
Expand All @@ -118,12 +128,20 @@ jobs:
`nn.Parameter`, optional tensors, variadic tensors, and tensors nested in
tuples/lists/mappings/dataclasses as tensor inputs. Require the Google-style
`Args` section and, when values are returned, `Returns` section to document:
- every tensor input and output's semantic shape and axis order, with
symbolic dimensions defined at first use (for example, `B` = batch, `S`
= sequence, `H` = hidden). Flag vague descriptions such as "embedding
tensor" or unexplained `[B, S, H]` notation. If arbitrary ranks or
leading dimensions are accepted, state that explicitly and identify the
constrained axes (for example, `[..., H]` with `H` = hidden size);
- every tensor input and output's semantic shape and axis order. Follow
the common format `hidden_states: Tensor of shape [batch, sequence,
hidden].` Use clear, conventional dimension names such as `batch`,
`sequence`, `tokens`, `hidden`, `heads`, `channels`, and `vocab` without
explaining them. Define non-obvious or model-specific dimension names at
first use. Flag vague descriptions such as "embedding tensor" or
layouts that do not follow this format. If arbitrary ranks or leading
dimensions are accepted, state that explicitly and identify the
constrained axes (for example, `hidden_states: Tensor of shape [...,
hidden], with arbitrary leading dimensions.`). When reporting a layout
documentation finding, include a concrete compliant rewrite in the
review; for example, `fused_qkv: Tensor of shape [batch, sequence, 3,
heads, head_dim], where axis 2 stores query, key, and value in that
order.`;
- nonstandard logical layout such as packed, padded, ragged, flattened,
interleaved, fused-QKV/GateUp, THD, or channels-first/channels-last, and
the before/after layout when the API transforms it;
Expand Down Expand Up @@ -163,6 +181,29 @@ jobs:
that class's nested `ModelCapabilities` dataclass (static pattern) or
inside the `get_capabilities(cls, config)` classmethod (dynamic
dispatch pattern).
- Checkpoint-free model initialization. When a PR adds or materially
changes model parameters, persistent buffers, initialization methods,
routing state, or a random-init recipe path, verify that construction
followed by the production initialization path leaves every tensor that
can affect the first forward/backward in a valid state without loading a
checkpoint:
- flag parameters or persistent buffers allocated with `torch.empty`, or
initialized to sentinel/placeholder values, unless every supported
model variant and stage (including auxiliary heads and MTP blocks)
overwrites them before use. A later checkpoint load is not a substitute
when checkpoint-free or random-init training is supported;
- require routing/index tables to satisfy the downstream dispatcher's
structural invariants before first use: indices must be in range and,
when the dispatcher requires it (for example DeepEP top-k routing),
expert IDs for each token must be distinct. An all-zero placeholder is
valid only when it satisfies the actual routing contract;
- require a focused test that disables checkpoint/base-model loading,
invokes the production initializer, and asserts finite initialized
parameters plus the relevant buffer/routing invariants. When this state
affects execution or gradients, also require a finite first
forward/backward on the smallest representative backend/topology; apply
the distributed test matrix below when a single-process backend cannot
exercise the contract.
- Low-precision dtype hazards in RoPE / precision-sensitive buffers. Flag
when a module registers a floating-point buffer used to build rotary
tables — `inv_freq`, `freqs_cis`, or precomputed `cos`/`sin` — and the
Expand Down
Loading