test: remove more documentation shape assertions - #9257
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (25)
💤 Files with no reviewable changes (24)
Included review availability: Your plan includes up to 12 reviews per rolling hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe pull request removes multiple documentation and policy contract tests, removes related Vitest watch mappings and unused imports, and updates the release-note procedure to use only ChangesDocumentation contract cleanup
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🟡 Moderate · up to This PR removes redundant documentation assertions while retaining executable behavior coverage and does not identify a production behavior change. Merge readiness remains incomplete until the required sensitive-path review or maintainer waiver is recorded. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-9257.docs.buildwithfern.com/nemoclaw |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite for the commit under review. Recommended E2E: None Manual-only E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
senthilr-nv
left a comment
There was a problem hiding this comment.
Maintainer review — latest PR commit f30064c778cf73540a3260815f0e8bba788d56e2
The prose-mirror cleanup is directionally sound, but two classes of deleted tests enforce behavior and safety contracts that npm run docs does not replace.
- Changelog release invariants lose their only validator.
test/changelog-docs.test.tschecks MDX-compatible SPDX syntax, release ordering and uniqueness, completeness of migrated history, literal command names, the absence of variant wrappers, and root-absolute links. Thenpm run docschain generates shared content, checks generated files and routes, and runs Fern validation; it does not enforce those release-history invariants. Keep these checks in a focused semantic validator, or move them into a canonical script invoked bynpm run docs, before changingdocs/CONTRIBUTING.mdto call the latter the replacement. - Security guidance loses behavior-linked regression evidence. The deleted operator-admin, credential-rotation, and policy-roundtrip suites protect concrete safety boundaries: no automatic
operator.adminapproval and exact-request recovery, keeping replacement credentials out of command text, narrow MCP routes, and preserving SSRF protection. Rendering and route checks do not verify those claims. Retain these safety assertions or replace them with an authoritative semantic validator tied to the corresponding behavior contracts.
This makes the current statement that retained security and documentation tests cover the stable boundaries inaccurate. The documentation receipt is current, and the product/runtime surface is otherwise unchanged. Cross-issue search found no competing or adjacent accepted scope. I did not run local validation, approve, or merge.
Summary
Remove documentation wording, navigation, migration-status, security-review prose, and skill-Markdown mirror tests that duplicate checked-in text. Retain executable coverage for parsers, generators, collectors, policy evaluators, workflow scripts, credential forms, JWTs, prompt integrity, and published-route validation.
Changes
npm run docsvalidation indocs/CONTRIBUTING.md.Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/CONTRIBUTING.mdnow uses the canonicalnpm run docschangelog validation. The complete test-removal diff was reviewed against the writing and documentation style rules.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpm run docs,npm run lint,npm run source-shape:check, andnpm run test:projects:checkpassed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not run locally; CI is the broad gate for this test-only deletion pass.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
Documentation
Tests