Skip to content

Refine agent contribution guidance#1488

Open
realAsma wants to merge 2 commits into
mainfrom
asma/modelopt_agent
Open

Refine agent contribution guidance#1488
realAsma wants to merge 2 commits into
mainfrom
asma/modelopt_agent

Conversation

@realAsma
Copy link
Copy Markdown
Contributor

@realAsma realAsma commented May 13, 2026

What does this PR do?

Type of change: documentation.

This PR adds repo-local production code guidelines for agents and simplifies the agent entrypoint.

The main intent is to keep .agents/README.md small: it links agents to the relevant repo docs and code areas instead of duplicating long command tables, architecture summaries, or policy text. The repo code and existing documentation remain the source of truth, and agents can follow the links to inspect the relevant parts for the task at hand.

The agent docs should primarily capture guidance that agents cannot reliably infer from source code alone, such as production coding principles, review expectations, PR readiness, and publishing constraints.

Key changes:

  • Add and refine production code guidelines in .agents/developer-guidelines.md.
  • Simplify .agents/README.md into a concise linked entrypoint for agent workflows.
  • Point agents to CONTRIBUTING.md, SECURITY.md, .github/PULL_REQUEST_TEMPLATE.md, pyproject.toml, .pre-commit-config.yaml, noxfile.py, and relevant ModelOpt code areas instead of restating that content.
  • Clarify test and pre-commit guidance by linking to the existing contributor docs.
  • Add focused pytest and nox examples to CONTRIBUTING.md for local validation.

Usage

N/A. Documentation-only change.

Testing

  • git diff --cached --check
  • Commit pre-commit hooks passed, including markdownlint-cli2.

Before your PR is "Ready for review"

Make sure you read and follow Contributor guidelines and your commits are signed (git commit -s -S).

Make sure you read and follow the Security Best Practices (e.g. avoiding hardcoded trust_remote_code=True, torch.load(..., weights_only=False), pickle, etc.).

  • Is this change backward compatible?: N/A
  • If you copied code from any other sources or added a new PIP dependency, did you follow guidance in CONTRIBUTING.md: N/A
  • Did you write any new necessary tests?: N/A
  • Did you update Changelog?: N/A
  • Did you get Claude approval on this PR?: N/A

Additional Information

Docs-only agent guidance update.

Summary by CodeRabbit

  • Documentation
    • Added comprehensive AI agent instructions and developer guidelines to support AI-assisted workflows.
    • Clarified test-running guidance with explicit pytest examples and retained nox usage notes.
    • Added tooling notes describing repository-level and local override practices for agent tools.
    • Added an "AI Agents" subsection in the README linking to the agent resources and guidance.

@realAsma realAsma requested a review from a team as a code owner May 13, 2026 21:08
@realAsma realAsma requested a review from kevalmorabia97 May 13, 2026 21:08
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9d3ca7fe-b44b-41e6-a9e5-a1a580831813

📥 Commits

Reviewing files that changed from the base of the PR and between 85fd1fb and ed53a34.

📒 Files selected for processing (1)
  • .agents/developer-guidelines.md
✅ Files skipped from review due to trivial changes (1)
  • .agents/developer-guidelines.md

📝 Walkthrough

Walkthrough

This PR centralizes ModelOpt's development guidance under a new .agents/ directory (README, tooling notes, developer guidelines), updates AGENTS.md and CLAUDE.md to point to that README, adds ignore rules for local override files, and clarifies test-running instructions in CONTRIBUTING.md and README.md.

Changes

Development Guidance Centralization

Layer / File(s) Summary
Agent instruction framework
.agents/README.md
New entry point for agent instructions documenting authoritative project guidance sources, test and pre-commit workflows, and Git workflow rules for push and PR creation.
Developer guidelines and rules
.agents/developer-guidelines.md
Production coding principles (design, readability, validation, documentation), testing practices (focused development and curated production tests), performant AI coding rules (avoid CPU-GPU syncs, rank-aware logging, no hidden global-state sync), and compatibility requirements for configs/checkpoints.
Tooling, pointers, and docs updates
.agents/TOOLING.md, .gitignore, AGENTS.md, CLAUDE.md, CONTRIBUTING.md, README.md
Tooling notes for agent overrides, added ignore patterns for local override files, AGENTS.md/CLAUDE.md updated to reference .agents/README.md, CONTRIBUTING.md clarifies "Writing and running tests" with a pytest example and updated nox guidance, and README adds an "AI Agents" pointer.

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Refine agent contribution guidance' directly and clearly summarizes the main change: updates to agent-related documentation and guidelines.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Security Anti-Patterns ✅ Passed Documentation-only PR. No Python security violations found: no unsafe torch.load, numpy.load, hardcoded trust_remote_code, eval/exec, nosec comments, or non-permissive dependencies.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch asma/modelopt_agent

Comment @coderabbitai help to get the list of available commands and usage tips.

@realAsma realAsma requested review from a team and danielkorzekwa and removed request for a team May 13, 2026 21:15
@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.60%. Comparing base (62401e1) to head (ed53a34).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1488      +/-   ##
==========================================
- Coverage   76.78%   76.60%   -0.18%     
==========================================
  Files         473      473              
  Lines       51413    51545     +132     
==========================================
+ Hits        39476    39488      +12     
- Misses      11937    12057     +120     
Flag Coverage Δ
unit 52.54% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Collaborator

@cjluo-nv cjluo-nv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bot review — DM the bot to share feedback.

Docs-only refactor that pulls agent guidance out of CLAUDE.md into a shared .agents/README.md plus developer-guidelines.md, with thin entrypoints for AGENTS.md and CLAUDE.md. Direction is good (single source of truth, link out instead of duplicate). A few improvement opportunities worth a look before merge — none are blockers:

  1. AGENTS.md uses Claude-specific @ include syntax. @.agents/README.md is interpreted as an inclusion directive by Claude Code, but AGENTS.md is the cross-tool convention (Codex, Cursor, Aider, etc.) and those tools will treat @.agents/README.md as literal text. Consider either inlining the content or using a plain markdown link plus a one-line "agents should read this file" instruction so non-Claude tools get the guidance.

  2. A few useful items from the old CLAUDE.md "Rules (Read First)" section don't survive the link-out. Specifically: the warning "do NOT substitute the harness's default ## Summary / ## Test plan format" when filling in the PR template, the "run /claude review for non-trivial PRs" guidance, "re-run tests locally after rebase", and "never attribute AI tools in sign-off". These are agent-specific behaviors that aren't restated in CONTRIBUTING.md, so linking there doesn't recover them. Worth adding a short "Agent-specific PR conventions" bullet list in .agents/README.md.

  3. CLAUDE.local.md override hint moved out of the shared file. Only CLAUDE.md mentions it now; agents reading via AGENTS.md.agents/README.md won't know about the local-overrides pattern. If the pattern is meant to be tool-agnostic (e.g. AGENTS.local.md), surface it in .agents/README.md; if it's Claude-only, that's fine as-is.

  4. Code-areas prose in .agents/README.md is less complete than the old table. It omits nas, peft, kernels, _deploy, and utils. Minor — but if the goal is "linked entrypoint", a one-line link to a fuller listing (or restoring the table) would be more useful than a partial enumeration.

  5. CONTRIBUTING.md nox example pulls in megatron-core. partial_unit-3.12(torch) installs megatron-core, which is heavyweight for what the section frames as "focused local validation". The pytest example just above already covers the lightweight case; consider whether the nox example here should point at a lighter session, or add a note that this session has heavier deps.

No correctness or licensing issues. Tests N/A (docs-only).

Comment thread AGENTS.md Outdated
@@ -0,0 +1,3 @@
# AGENTS.md

@.agents/README.md
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bot comment.

@.agents/README.md is Claude Code's include syntax. AGENTS.md is the cross-tool spec (Codex, Cursor, Aider, etc.) and those tools will render this as literal text rather than expanding the include — agents using them won't see the guidance. Consider inlining the content here, or replacing with a plain markdown link plus a sentence telling the agent to read it (e.g. Agents working in this repo should read [.agents/README.md](.agents/README.md) before making changes.).

Comment thread .agents/README.md Outdated
- **Contributing and PRs:** Read `CONTRIBUTING.md` for commit conventions,
DCO sign-off, signing, PR expectations, and review requirements.

- **PR readiness:** Before opening or marking a PR ready for review, read
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bot comment.

A few items from the old CLAUDE.md Rules section don't carry over by linking to CONTRIBUTING.md: the warning to fill in .github/PULL_REQUEST_TEMPLATE.md verbatim and not substitute the harness's default ## Summary / ## Test plan format, running /claude review for non-trivial PRs, re-running tests after rebase, and "never attribute AI tools in the sign-off line". These are agent-specific behaviors not restated in CONTRIBUTING.md. Worth keeping a short "Agent PR conventions" bullet list here.

Comment thread CLAUDE.md Outdated
| Example/integration tests | `tests/examples/` | Integration tests for examples; see `tests/examples/README.md` |
| Pre-commit / lint | `.pre-commit-config.yaml` | ruff, mypy, clang-format, license headers, bandit |
| Coverage | `pyproject.toml` | 70% minimum on `modelopt/*` |
If `CLAUDE.local.md` exists, read it after `.agents/README.md` for local overrides.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bot comment.

The CLAUDE.local.md override hint is only visible to Claude users now. If the local-overrides pattern is meant to be tool-agnostic, consider moving (or duplicating) it into .agents/README.md so agents reaching it via AGENTS.md also know about it.

Comment thread .agents/README.md Outdated
`modelopt.torch.speculative` covers speculative decoding; and
`modelopt.torch.export` covers export flows. `modelopt.torch.prune`,
`modelopt.torch.distill`, and `modelopt.torch.sparsity` cover other torch
algorithms. `modelopt.onnx`, `modelopt.deploy`, and `modelopt.recipe` cover
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bot comment.

This enumeration drops nas, peft, kernels, _deploy, and utils that the old table covered. Either restore them, or replace the prose with a single link to a fuller listing — a partial enumeration is a bit of a trap (agents may assume there's nothing else).

Comment thread CONTRIBUTING.md
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:

```bash
nox -s "partial_unit-3.12(torch)"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bot comment.

partial_unit-3.12(torch) installs megatron-core as part of the session, which is fairly heavy for what this section frames as "focused local validation". The plain pytest example above already covers the lightweight path; consider either pointing at a lighter nox session or noting the heavier deps so users aren't surprised.

Signed-off-by: realAsma <akuriparambi@nvidia.com>
@realAsma realAsma force-pushed the asma/modelopt_agent branch from 7cc5739 to 85fd1fb Compare May 14, 2026 01:03
@realAsma realAsma requested a review from a team as a code owner May 14, 2026 01:03
Signed-off-by: realAsma <akuriparambi@nvidia.com>
@realAsma
Copy link
Copy Markdown
Contributor Author

Bot comment.

Review follow-up after the latest head:

  • The AGENTS.md include concern is stale: AGENTS.md and CLAUDE.md are now symlink entrypoints to .agents/README.md, so the old Claude-only @.agents/README.md hunk is gone.
  • Keeping .agents/README.md intentionally lean. The shared file links to the PR, signing, testing, and pre-commit guidance instead of duplicating every old Claude-specific rule; tool-specific setup belongs in .agents/TOOLING.md.
  • CLAUDE.local.md is intentionally documented in .agents/TOOLING.md because local override behavior differs by tool and is not always-loaded repo guidance.
  • The code-area comment was addressed by removing the partial package enumeration and using broad source/test/docs orientation instead.
  • The nox concern is addressed in CONTRIBUTING.md: it now distinguishes lightweight direct pytest validation from broader nox validation and calls out the heavier megatron-core dependency for partial_unit-3.12(torch).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants