fix(skills): enforce PR comparator merge eligibility

[fallintoplace]

Summary

Make the PR comparator derive merge eligibility from the complete Tier 0 gate set before rendering a verdict. Malformed gate data, contradictory modes, and ineligible winners now fail closed instead of producing an unsafe MERGE PR recommendation.

Changes

• Define one canonical seven-gate Tier 0 schema, including contributor compliance, and use it for validation and scorecard rendering.
• Reject missing, unknown, or non-boolean gate values before emitting Markdown.
• Derive happy and degraded modes from the eligible PR set instead of trusting caller-supplied mode data.
• Reserve winner for fully merge-eligible PRs and use closest_to_ready for degraded-mode salvage guidance.
• Align the comparator instructions, tiebreakers, and verdict template with the enforced renderer contract.
• Add black-box regression coverage for ineligible winners, malformed gates, contradictory modes, valid merges, and degraded recommendations.

Type of Change

• Code change (feature, bug fix, or refactor)
• Code change with doc updates
• Doc only (prose changes, no code sample modifications)
• Doc only (includes code sample changes)

Quality Gates

• Tests added or updated for changed behavior
• Existing tests cover changed behavior — justification:
• Tests not applicable — justification:
• Docs updated for user-facing behavior changes
• Docs not applicable — justification: this changes an internal maintainer workflow; its operator-facing skill, tiebreaker, and verdict-template documentation are updated in the same change, with no public docs/ impact.
• 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: reviewed the renderer as an untrusted-input boundary; one canonical gate list drives validation and display, and regression tests verify invalid specifications exit nonzero without emitting a verdict.
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

• PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
• Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
• Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: npx vitest run --project integration test/pr-comparator-render-verdict.test.ts test/maintainer-skills-policy.test.ts test/skills/check-gates-compliance.test.ts — 54 tests passed across 3 files.
• 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; this is isolated to the internal comparator renderer and workflow contract.
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• npm run docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

---

Signed-off-by: Minh Vu vuhoangminh97@gmail.com

Summary by CodeRabbit

• Improvements

  • PR comparison verdicts now follow stricter, fully deterministic Tier 0 eligibility rules (now based on seven gates).
  • Verdict rendering no longer trusts caller-provided mode/winner and rejects invalid or incomplete specs up front.
  • Merge recommendations are issued only for fully eligible PRs; degraded mode reports only salvageable candidates.

• Documentation

  • Updated Tier 0 gate descriptions and the verdict template’s scorecard to match the new gate set and guidance.

• Tests

  • Added new automated tests covering spec validation, winner/closest-to-ready safeguards, and degraded salvage behavior.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR makes Tier 0 eligibility deterministic across seven gates, adds strict renderer-side specification validation, derives verdict mode and salvage candidates from gate values, updates verdict guidance and scorecard output, and adds tests for validation, rendering, degraded salvage, and documentation alignment.

Changes

Tier 0 verdict flow

Layer / File(s)	Summary
Seven-gate eligibility and verdict-selection rules .agents/skills/nemoclaw-maintainer-pr-comparator/SKILL.md, .agents/skills/nemoclaw-maintainer-pr-comparator/checks/tier-0-gates.md, .agents/skills/nemoclaw-maintainer-pr-comparator/tiebreakers.md, .agents/skills/nemoclaw-maintainer-pr-comparator/scripts/collect-gates.sh	Defines seven required boolean gates, including Advisor merge-as-is and automated-review thread checks; derives mode from eligible PRs and constrains winner and closest_to_ready.
Renderer validation and derived control flow .agents/skills/nemoclaw-maintainer-pr-comparator/scripts/render-verdict.py	Validates specification shape and gate values, derives mode and salvage candidates, validates selections, and renders updated gate statuses and degraded messages.
Verdict output and alignment coverage .agents/skills/nemoclaw-maintainer-pr-comparator/templates/verdict.md, test/pr-comparator-render-verdict.test.ts	Updates scorecard gate rows and tests valid rendering, invalid specifications, mode derivation, salvage behavior, and skill documentation alignment.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant validate_spec
  participant render_verdict
  Caller->>validate_spec: submit verdict specification
  validate_spec->>validate_spec: validate seven Tier 0 gates
  validate_spec->>validate_spec: derive mode and closest_to_ready
  validate_spec->>render_verdict: pass validated results
  render_verdict->>Caller: emit verdict or validation error

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly reflects the main change: tightening PR comparator merge eligibility rules.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

test/pr-comparator-render-verdict.test.ts (1)

123-135: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Doc-alignment test locks in exact SKILL.md wording rather than behavior.

This test asserts literal substrings from SKILL.md (e.g. "set \winner` only to a PR in that set"`). A future wording tweak that preserves the same semantics would break this test without any behavioral regression, which is the kind of source-text lock-in the path instructions caution against.

As per path instructions: "Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/pr-comparator-render-verdict.test.ts` around lines 123 - 135, The test
keeps exact SKILL.md wording instead of validating observable behavior. Refactor
“keeps the generation instructions aligned with renderer eligibility” to
exercise the public comparator/rendering boundary and assert the resulting
eligibility, winner, closest-to-ready, and nonzero-exit outcomes; remove literal
substring assertions against SKILL.md.

Source: Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/pr-comparator-render-verdict.test.ts`:
- Around line 123-135: The test keeps exact SKILL.md wording instead of
validating observable behavior. Refactor “keeps the generation instructions
aligned with renderer eligibility” to exercise the public comparator/rendering
boundary and assert the resulting eligibility, winner, closest-to-ready, and
nonzero-exit outcomes; remove literal substring assertions against SKILL.md.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4f31e202-197b-45d2-950b-3c0f5017c518

📥 Commits

Reviewing files that changed from the base of the PR and between 5be42a9 and b4b7e3e.

📒 Files selected for processing (5)

• .agents/skills/nemoclaw-maintainer-pr-comparator/SKILL.md
• .agents/skills/nemoclaw-maintainer-pr-comparator/scripts/render-verdict.py
• .agents/skills/nemoclaw-maintainer-pr-comparator/templates/verdict.md
• .agents/skills/nemoclaw-maintainer-pr-comparator/tiebreakers.md
• test/pr-comparator-render-verdict.test.ts

[github-actions]

E2E Advisor Recommendation

Required E2E: None
Optional E2E: None

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: target/main
Head: HEAD
Confidence: high

Required E2E

• None. No existing NemoClaw E2E job is recommended. The PR does not touch installer/onboarding, sandbox lifecycle, credentials, security boundaries, network policy, inference routing, deployment, or live assistant user flows. The risk plan requires no E2E jobs, and the changed renderer behavior is covered by a targeted unit test.

Optional E2E

• None.

New E2E recommendations

• None.

[github-actions]

E2E Target Recommendation

Required E2E targets: None
Optional E2E targets: None

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: target/main
Head: HEAD
Confidence: high

Required E2E targets

• None. Changes are limited to maintainer PR-comparator skill files and a non-E2E unit test outside test/e2e; they do not affect the NemoClaw E2E target workflow, registry, runtime support, fixtures, or target-relevant application paths.

Optional E2E targets

• None.

Relevant changed files

• None.

[cv]

Maintainer follow-up at exact head 1b03474:

• Corrected the PR description from six to seven canonical Tier-0 gates.
• Synced current main with a signed, conflict-free merge.
• Merged-head verification passed: 55 focused integration tests, CLI typecheck, repository-pinned ShellCheck, Python syntax, and normal hooks. The merge commit is signed, DCO-signed-off, and GitHub Verified.
• The CodeRabbit wording-lock nit does not require a change: black-box renderer tests already exercise eligible/ineligible winner, malformed-gate, contradictory-mode, and degraded-salvage outcomes; the small SKILL.md assertion separately prevents the executable agent instructions from drifting from the renderer’s exact seven-key schema.
• Public docs review found no update necessary; the internal skill files changed here are the authoritative operator guidance.
• No live E2E target applies. Exact-head CI is approved and running. A trusted GPT/Nemotron dispatch will follow after the currently running fix(backup): name the per-dir failure cause in backup failure messages #6631 advisor completes, avoiding the repository’s cross-PR concurrency cancellation.

No merge action taken.

[fallintoplace]

Let me know if there is anything I need to do that comes up.

[cv]

Approved at exact head 1b03474. GPT-5.5 and Nemotron 3 Ultra both returned high-confidence merge_as_is with empty finding ledgers; the runtime-validation suggestions are non-blocking because the real renderer boundary is exercised by focused black-box tests and the exact-head standard CI matrix is green. E2E Advisor requires no targets, CodeRabbit has no unresolved review threads, DCO passes, and every commit is GitHub Verified. No merge action taken.