docs(contributing): add engineering behavior guidance - #6521
Conversation
E2E Advisor RecommendationRequired E2E: None Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
|
E2E Target RecommendationRequired E2E targets: None Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
📝 WalkthroughWalkthroughThis PR updates AGENTS.md and CONTRIBUTING.md with expanded contributor guidance. AGENTS.md gains a new engineering-posture checklist step, while CONTRIBUTING.md adds instructions on defining success criteria, scoping changes, evidence-driven verification, and documenting escaped defects. ChangesContribution documentation updates
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
PR Review Advisor (Nemotron Ultra) — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
PR Review Advisor — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
|
Advisor follow-up:
|
cv
left a comment
There was a problem hiding this comment.
I don't think this should be a skill that optionally loads, but a mention in AGENTS.md and the fuller text permeating CONTRIBUTING.md where applicable. Ideally this isn't even a section in them, but the posture and principles are spread throughout (sans repetition, of course)
|
Addressed the requested design change from this review at exact head
The GPT and Nemotron advisor items asking to restore the skill follow the issue's original implementation wording, but that scope is intentionally superseded by the maintainer review above. Restoring the optional skill would contradict the requested always-on design. Validation on this head: 36/36 focused tests, 19/19 repository skills, local Markdown links, |
<!-- markdownlint-disable MD041 --> ## Summary Adds concise, always-applicable engineering behavior guidance for NemoClaw coding work. In response to maintainer review, the detailed posture now lives in the existing contributor workflow instead of an optional skill: assumptions and ambiguity are made reviewable, changes stay minimal and issue-scoped, outcomes receive narrow evidence, and QA-escaped defects address both the product root cause and the detection gap. ## Related Issue Fixes NVIDIA#6515. ## Changes - Add one compact pointer to the contributor engineering posture in root `AGENTS.md`. - Integrate planning, scope, verification, and escaped-defect guidance into the relevant existing sections of `CONTRIBUTING.md`. - Remove the proposed optional skill and its catalog changes, following maintainer feedback that the posture should apply to every coding task. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: This is contributor-policy prose with no executable product behavior. Existing skill/frontmatter policy tests pass 36/36, and the repository skill validator passes all 19 skills after removal of the optional skill. - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: The required documentation review confirmed no end-user NemoClaw behavior or `docs/` page changes; this is internal contributor and coding-agent guidance. - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run --project integration test/skills-frontmatter.test.ts test/maintainer-skills-policy.test.ts` passed 36/36; `validate_repo_skills.sh --repo .` passed all 19 skills; local links passed for `AGENTS.md` and `CONTRIBUTING.md`. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not applicable to this focused prose-only change. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — build passed with 0 errors; Fern reported the existing unauthenticated redirect-check and light-mode contrast warnings. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: cjagwani <cjagwani@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated contributor guidance with clearer expectations for small, scoped changes, explicit assumptions, and observable success criteria. * Expanded instructions for testing, regression evidence, and handling escaped defects. * Added stronger pull request guidance to keep work focused, avoid unrelated cleanup, and document any scope changes. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Adds concise, always-applicable engineering behavior guidance for NemoClaw coding work. In response to maintainer review, the detailed posture now lives in the existing contributor workflow instead of an optional skill: assumptions and ambiguity are made reviewable, changes stay minimal and issue-scoped, outcomes receive narrow evidence, and QA-escaped defects address both the product root cause and the detection gap.
Related Issue
Fixes #6515.
Changes
AGENTS.md.CONTRIBUTING.md.Type of Change
Quality Gates
docs/page changes; this is internal contributor and coding-agent guidance.Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run --project integration test/skills-frontmatter.test.ts test/maintainer-skills-policy.test.tspassed 36/36;validate_repo_skills.sh --repo .passed all 19 skills; local links passed forAGENTS.mdandCONTRIBUTING.md.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not applicable to this focused prose-only change.npm run docsbuilds without warnings (doc changes only) — build passed with 0 errors; Fern reported the existing unauthenticated redirect-check and light-mode contrast warnings.Signed-off-by: cjagwani cjagwani@nvidia.com
Summary by CodeRabbit