Skip to content

Commit 7cc5739

Browse files
committed
Refine agent contribution guidance
Signed-off-by: realAsma <akuriparambi@nvidia.com>
1 parent 7f24341 commit 7cc5739

3 files changed

Lines changed: 89 additions & 53 deletions

File tree

.agents/README.md

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,18 @@ here.
1111

1212
- **Code areas:** `modelopt.torch.opt` is shared optimization infrastructure;
1313
`modelopt.torch.quantization` covers quantization algorithms;
14-
`modelopt.torch.export` covers export flows; `modelopt.torch.prune`,
15-
`modelopt.torch.distill`, `modelopt.torch.sparsity`, and
16-
`modelopt.torch.speculative` cover other torch algorithms. `modelopt.onnx`,
17-
`modelopt.deploy`, and `modelopt.recipe` cover ONNX, deployment, and recipes.
14+
`modelopt.torch.speculative` covers speculative decoding; and
15+
`modelopt.torch.export` covers export flows. `modelopt.torch.prune`,
16+
`modelopt.torch.distill`, and `modelopt.torch.sparsity` cover other torch
17+
algorithms. `modelopt.onnx`, `modelopt.deploy`, and `modelopt.recipe` cover
18+
ONNX, deployment, and recipes.
1819

1920
- **Contributing and PRs:** Read `CONTRIBUTING.md` for commit conventions,
2021
DCO sign-off, signing, PR expectations, and review requirements.
2122

23+
- **PR readiness:** Before opening or marking a PR ready for review, read
24+
`.github/PULL_REQUEST_TEMPLATE.md` and satisfy the checklist.
25+
2226
- **Security:** Read `SECURITY.md` before changing external input handling,
2327
serialization, subprocess use, dependencies, or user-facing behavior.
2428

@@ -28,9 +32,20 @@ here.
2832
- **License headers:** New Python, C++, and CUDA files need the SPDX license
2933
header from `LICENSE_HEADER`.
3034

31-
- **Running tests:** Use `pyproject.toml`, `noxfile.py`, and `tests/` to choose
32-
focused tests for the changed area.
35+
- **Running tests:** Follow the
36+
[test instructions](../CONTRIBUTING.md#-writing-and-running-tests) in
37+
`CONTRIBUTING.md`. For fast initial iteration, choose focused tests for the
38+
changed area from `tests/`.
39+
40+
- **Running pre-commit:** Follow the
41+
[pre-commit hook instructions](../CONTRIBUTING.md#pre-commit-hooks) in
42+
`CONTRIBUTING.md`. Hooks may modify files; review and re-stage those changes
43+
before committing.
44+
45+
## Git Workflow
3346

34-
- **Running pre-commit:** Use `.pre-commit-config.yaml` for lint, format, type,
35-
license, and security hooks. Hooks may modify files; review and re-stage those
36-
changes before committing.
47+
- **Never `git push` without explicit approval in the current turn.** Commit
48+
locally is fine; publishing to a remote is not.
49+
- After `git commit`, stop and wait for the user to
50+
say "push", "publish", "ship", or equivalent before running `git push`,
51+
`gh pr create`, or any push-option flags like `-o merge_request.create`.

.agents/developer-guidelines.md

Lines changed: 53 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,53 @@
1-
# Minimal-Developer Principles
1+
# Coding Principles
22

3-
Guidelines for writing production code. Key values: simplicity, minimalism, and
4-
elegance.
3+
Guidelines for production code in ModelOpt. Key values: simplicity, minimalism,
4+
and elegance.
55

6-
## Philosophy
6+
## Principles
77

8-
- **Be Surgical, Minimal edits.** Touch only what the task requires. No speculative
9-
refactors, drive-by cleanup, or half-finished implementations.
10-
- **Good design simplifies code.** Good architecture makes code easier to read
11-
and easier to change. Use abstractions that put code at the right level, and
12-
keep designs extensible for known needs without over-engineering for
13-
speculative future cases. Heavy branching and conditional logic are design
14-
smells because they are hard to read and prone to bugs.
8+
- **Design for simplicity.** Before planning non-trivial changes, choose the
9+
design that keeps code easiest to read and change. Put behavior at the right
10+
level, tie extensibility to known needs, and treat heavy branching or
11+
conditional logic as bad design smells.
12+
- **Be surgical.** Touch the code required to solve the actual problem, whether
13+
that is one line or a broader design change. Avoid speculative refactors,
14+
drive-by cleanup, unrelated rewrites, and half-finished implementations.
15+
- **Fix root causes.** Prefer the right fix over the most local patch. Do not
16+
paper over symptoms with temporary fixes unless the temporary nature and
17+
follow-up are explicit.
1518
- **Respect ownership and layering.** Keep behavior in the layer that owns it.
1619
Parent abstractions should contain shared contracts and shared behavior, not
1720
child-specific special cases.
18-
- **Prefer clean extension over branching.** When behavior varies, use explicit
19-
extension points such as strategies, adapters, callbacks, or overrides. Update
20-
a parent or shared base when it is the right extension point, but do not add
21-
extension points for speculative future cases.
22-
- **Use helpers for duplication and right-level abstraction.** Add helpers when
23-
they remove duplicated code across multiple places or let higher-level code
24-
use helpful names while low-level details stay abstracted away.
25-
- **Use meaningful names.** Methods, variables, objects, and helpers should make
26-
intent clear at the point of use.
27-
- **Code should read like prose.** Well-written code is self-explanatory and
28-
elegant. Keep high-level orchestration clear, and move low-level mechanics
29-
into well-named helpers when that improves readability.
30-
- **Critical code first.** Order files so the most important or user-facing code
31-
appears before helper details when local conventions allow it.
32-
- **Validate at boundaries, then trust invariants.** Check untrusted inputs at system boundaries such as user input, external APIs, files, networks, and process boundaries. After validation, internal code should trust its types and invariants instead of adding defensive checks for states that should be impossible.
33-
- **Comments explain why.** Code is the source of truth for what happens and
21+
- **Use abstractions to simplify.** Add helpers, base classes, registries,
22+
adapters, plugins, or other abstractions when they remove real duplication,
23+
clarify ownership, or put behavior at the right level. Do not add abstractions
24+
for speculative future cases.
25+
- **Prefer extension over branching.** When behavior varies, use explicit
26+
extension points such as adapters, registries, callbacks, plugins, or
27+
overrides. Update a parent or shared base when it is the right extension
28+
point. Do not add extension points for speculative future cases.
29+
- **Make code readable at the point of use.** Names, types, and structure should
30+
make intent clear. Keep high-level orchestration clear, and move low-level
31+
mechanics into well-named helpers when that improves readability.
32+
- **Put critical code first.** Order files so the most important or user-facing
33+
code appears before helper details when local conventions allow it.
34+
- **Validate outside input once.** Check user input, files, network responses,
35+
and external API results at the edge. Keep internal code simple instead of
36+
repeatedly checking for impossible states.
37+
- **Comments explain why, concisely.** Code is the source of truth for what happens and
3438
how. Add comments only when the reason is not obvious. Redundant comments are
3539
a maintainability burden. If a comment feels necessary, first check whether
3640
better design or naming would make the code explain itself.
37-
- **Preserve existing comments.** Minimalism applies to new comments; do not
38-
delete existing comments casually. Production code is shared by many
39-
developers, and unnecessary changes to others' code create avoidable review and
40-
approval overhead.
41-
- **Docstrings scale with API level.** Higher-level and user-visible APIs
42-
deserve useful docstrings, including examples when helpful. Lower-level
43-
internals should use minimal docstrings, or none, when well-named identifiers
44-
are enough.
45-
- **Remove dead code.** Delete unused imports, unreachable branches, and
46-
obsolete placeholders.
47-
- **Use workspace-relative paths** in commands and file references unless an
48-
absolute path is needed to disambiguate.
41+
- **Apply comment guidance to new comments only.** Use these standards only when adding
42+
new comments. Do not rewrite or delete existing comments as cleanup;
43+
- **Scale documentation to the API.** Higher-level and user-visible APIs deserve
44+
useful docstrings, including examples when helpful. Lower-level internals need
45+
docstrings only when names, types, and structure are not enough.
46+
- **Remove dead code.** Delete unused imports, unreachable branches, obsolete
47+
placeholders, stale TODOs, and debug code when they are part of the touched
48+
behavior.
49+
- **Use workspace-relative paths.** Use relative paths in commands and file
50+
references unless an absolute path is needed to disambiguate.
4951

5052
## Testing
5153

@@ -62,10 +64,19 @@ elegance.
6264

6365
- **Avoid stray CPU-GPU syncs.** Tensor metadata such as `tensor.shape` is safe
6466
to read, but scalar extraction or CPU transfers such as `tensor.item()`,
65-
`float(tensor)`, `bool(tensor)`, `tensor.cpu()`, and `tensor.numpy()` can force
66-
CPU-GPU synchronization. Keep computation on GPU unless the CPU actually needs
67-
the value.
67+
`float(tensor)`, `bool(tensor)`, `tensor.cpu()`, `tensor.numpy()`, etc. can
68+
force CPU-GPU synchronization. Keep computation on GPU unless the CPU actually
69+
needs the value.
6870
- **Use rank-aware logging.** Default to `print_rank_0` instead of `print` and
6971
`warn_rank_0` instead of generic warnings. Use per-rank output only when each
7072
process needs to report distinct state. Generic prints and warnings clog
7173
distributed logs.
74+
- **Respect distributed invariants.** Avoid hidden synchronization, global state,
75+
per-rank file races, or assumptions that only hold on a single process.
76+
77+
## Compatibility
78+
79+
- **Preserve config and checkpoint compatibility.** Treat ModelOpt config schemas
80+
and checkpoint formats as persisted contracts. When changing configs such as
81+
`QuantizeConfig`, maintain backward compatibility with previous ModelOpt
82+
checkpoints unless a breaking change is explicit and intentionally handled.

CONTRIBUTING.md

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ If you are an external contributor, seek guidance from `@NVIDIA/modelopt-setup-c
7979

8080
See [`modelopt/torch/quantization/utils/calib_utils.py`](./modelopt/torch/quantization/utils/calib_utils.py) for an example of the correct license header format.
8181

82-
## 📝 Writing tests
82+
## 📝 Writing and running tests
8383

8484
We use [pytest](https://docs.pytest.org/) for all tests. For any new features / examples, make sure to add tests and that the coverage check in your PR passes. The tests are organized into the following directories:
8585

@@ -89,7 +89,17 @@ We use [pytest](https://docs.pytest.org/) for all tests. For any new features /
8989
- `tests/gpu_trtllm`: Fast GPU-based unit tests for the core ModelOpt library for TensorRT-LLM features. In most cases, they should not take more than a few seconds to run.
9090
- `tests/examples`: Integration tests for ModelOpt examples. They should not take more than a few minutes to run. Please refer to [example test README](./tests/examples/README.md) for more details.
9191

92-
Please refer to [noxfile.py](./noxfile.py) for more details on how to run the tests and their dependencies.
92+
For focused local validation, run `pytest` directly on the relevant test path. For example:
93+
94+
```bash
95+
pytest tests/unit/torch/quantization
96+
```
97+
98+
For standard repo sessions and dependency setup, use [noxfile.py](./noxfile.py). Run `nox -l` to list available sessions, then run the matching session with `nox -s <session>`. For example, `partial_unit` covers the broader torch unit test suite, including areas such as quantization:
99+
100+
```bash
101+
nox -s "partial_unit-3.12(torch)"
102+
```
93103

94104
## ✍️ Signing your work
95105

0 commit comments

Comments
 (0)