feat(advisor): review localized patches at source

[cv]

Summary

Adds a source-of-truth review surface to the PR Review Advisor so fallback, recovery, tolerant parsing, monkeypatching, and other localized workaround behavior is reviewed against the actual source boundary. The advisor now emits structured source-of-truth results, detects localized patch signals from added diff lines, and turns missing follow-up into actionable architecture findings.

Changes

• Added sourceOfTruthReview to the PR Review Advisor JSON schema and normalized result contract.
• Added deterministic localized patch signal detection for fallback/recovery paths, runtime monkeypatching, and silent/defaulted error handling.
• Updated the advisor rubric to require invalid-state, source-boundary, source-fix, regression-test, and removal-condition analysis.
• Rendered source-of-truth details in the detailed review artifact and documented the new advisor responsibility.
• Added tests for prompt coverage, signal detection, automatic source-of-truth findings, rendering, and schema validation.

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)

Verification

• npx prek run --all-files passes
• npm test passes
• Tests added or updated for new or changed behavior
• No secrets, API keys, or credentials committed
• Docs updated for user-facing behavior changes
• make 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)

Additional verification run:

• npx vitest run test/pr-review-advisor.test.ts passed.
• npm run build:cli passed.
• git diff --check passed.
• npx prek run --all-files failed before hooks ran: Error fetching release: self-signed certificate in certificate chain.
• npm test was run and failed in unrelated existing areas, including test/ssrf-parity.test.ts missing nemoclaw/dist/blueprint/private-networks.js, gateway/status CLI tests hitting TypeError: shields.getShieldsPosture is not a function, and deploy validation expectation drift.

---

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

• New Features

  • Source-of-truth review now evaluates localized workarounds/fallbacks and surfaces follow-up requirements.
  • New detailed review artifact provides expanded acceptance, security, and source-of-truth analysis.

• Bug Fixes

  • Architecture warnings added when source-of-truth reviews are missing or require follow-up.

• Tests

  • Added tests covering localized patch signal detection and source-of-truth follow-up behavior.

• Documentation

  • Updated README to document source-of-truth review capability.

[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]

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: 4129e181-b423-4276-8c15-8b4c651ba6c6

📥 Commits

Reviewing files that changed from the base of the PR and between 0ffcd62 and 48f9eaa.

📒 Files selected for processing (2)

• test/pr-review-advisor.test.ts
• tools/pr-review-advisor/analyze.mts

---

📝 Walkthrough

Walkthrough

Adds a source-of-truth review dimension: types and schema, diff-based localized-patch detection, LLM rubric updates, result normalization that injects findings for missing follow-up, detailed markdown rendering, and tests/docs updates.

Changes

Source-of-truth review feature

Layer / File(s)	Summary
Type contracts and JSON schema tools/pr-review-advisor/analyze.mts, tools/pr-review-advisor/schema.json	SourceOfTruthStatus enum and SourceOfTruthReview structure define per-surface review items with status, invalid-state, boundaries, conditions, and evidence. ReviewAdvisorResult is extended with sourceOfTruthReview array. JSON schema adds required sourceOfTruthReview field with strict property definitions and additionalProperties: false.
Localized patch signal detection tools/pr-review-advisor/analyze.mts, test/pr-review-advisor.test.ts	detectLocalizedPatchSignals(diff) scans unified diffs for regex-driven patterns (fallback/recovery, runtime monkeypatching, silent error handling) and records matching surfaces with file/line context and evidence. Test validates detection behavior on constructed diff patterns.
System prompt source-of-truth review rubric tools/pr-review-advisor/analyze.mts, test/pr-review-advisor.test.ts	System prompt rubric is extended with source-of-truth review guidance block instructing the advisor on pattern inspection, invalid-state validation, and rules for representing missing/needs_followup items as findings. Tests assert updated prompt content.
Result normalization and finding injection tools/pr-review-advisor/analyze.mts, test/pr-review-advisor.test.ts	normalizeReviewResult sanitizes sourceOfTruthReview items and calls addSourceOfTruthFindings to inject warning findings for items marked missing or needs_followup not already covered. Unavailable result updated to include empty sourceOfTruthReview field. Tests verify finding injection and deduplication behavior.
Detailed review markdown rendering tools/pr-review-advisor/analyze.mts, test/pr-review-advisor.test.ts	renderDetailedReview adds a new "Source-of-truth review" section listing each localized surface with its structured fields (status, invalid-state, boundary, why-not-source-fix, regression test, removal condition, evidence). Test assertions verify section presence and trusted-code boundary content.
Test fixtures and documentation test/pr-review-advisor.test.ts, tools/pr-review-advisor/README.md	Test imports updated to include signal detection; metadata fixture extended with empty localizedPatchSignals; validResult fixture populated with trusted-code boundary source-of-truth review entry. README documents source-of-truth review capability and adds pr-review-advisor-detailed-review.md artifact to outputs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I sniff the diffs where patches hide,
Small bandage fixes tucked inside—
I flag the spots that lack a source,
So follow-up can stay the course.
Hooray for truthful, traced repair!

🚥 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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and concisely describes the main feature: adding source-of-truth review for localized patches, which is the primary change across all modified files.
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.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/pr-advisor-source-of-truth-review

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

---

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

[github-actions]

E2E Advisor Recommendation

Required E2E: None
Optional E2E: None

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• None. No NemoClaw E2E is recommended. The PR only changes CI advisory tooling for PR review output plus its schema, docs, and unit tests; it cannot directly affect real assistant runtime behavior, installer/onboarding, sandbox lifecycle, credentials, network policy, inference routing, or deployment flows covered by existing E2E jobs.

Optional E2E

• None.

New E2E recommendations

• None.

[github-actions]

E2E Scenario Advisor Recommendation

Required scenario E2E: None
Optional scenario E2E: None

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

• None. No scenario workflow, scenario metadata, scenario runtime, or validation-suite files changed.

Optional scenario E2E

• None.

Relevant changed files

• None.

[github-actions]

PR Review Advisor

Findings: 0 needs attention, 1 worth checking, 0 nice ideas
Since last review: 1 prior item resolved, 0 still apply, 1 new item found

Review findings

🛠️ Needs attention

• None.

🔎 Worth checking

• Source-of-truth de-duplication is too broad (tools/pr-review-advisor/analyze.mts:833): The generated source-of-truth finding is skipped whenever any existing finding title, description, or evidence contains the review surface string. That does not prove the existing finding fully covers the missing or needs_followup source-of-truth requirement, so an unrelated finding that mentions the same surface can still suppress the mandatory architecture finding.
  • Recommendation: Tighten the coverage check to require explicit source-of-truth content, such as invalid state/source boundary/removal condition language, or always inject the generated finding and rely on rendering to tolerate duplicates. Add a regression test where an unrelated finding mentions the same surface but does not cover the source-of-truth follow-up.
  • Evidence: addSourceOfTruthFindings skips injection when `${finding.title}\n${finding.description}\n${finding.evidence}` includes `review.surface.toLowerCase()`, even though the prompt only permits skipping when the issue is already fully covered by a more specific finding.

🌱 Nice ideas

• None.

Since last review details

Current findings:

• Source-of-truth de-duplication is too broad (tools/pr-review-advisor/analyze.mts:833): The generated source-of-truth finding is skipped whenever any existing finding title, description, or evidence contains the review surface string. That does not prove the existing finding fully covers the missing or needs_followup source-of-truth requirement, so an unrelated finding that mentions the same surface can still suppress the mandatory architecture finding.
  • Recommendation: Tighten the coverage check to require explicit source-of-truth content, such as invalid state/source boundary/removal condition language, or always inject the generated finding and rely on rendering to tolerate duplicates. Add a regression test where an unrelated finding mentions the same surface but does not cover the source-of-truth follow-up.
  • Evidence: addSourceOfTruthFindings skips injection when `${finding.title}\n${finding.description}\n${finding.evidence}` includes `review.surface.toLowerCase()`, even though the prompt only permits skipping when the issue is already fully covered by a more specific finding.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

[coderabbitai]

Actionable comments posted: 2

🤖 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.

Inline comments:
In `@tools/pr-review-advisor/analyze.mts`:
- Around line 459-461: The code currently treats any line starting with "+++" as
a file header and skips it, which also drops real added lines like
"+++fallback"; update the header-detection logic in analyze.mts (the block that
checks rawLine) to only treat a line as a diff file header when it starts with
"+++ " (three pluses followed by a space) or is exactly "+++" if you want to be
extra safe; replace the existing rawLine.startsWith("+++") check with
rawLine.startsWith("+++ ") (and optionally || rawLine === "+++") so that valid
added lines beginning with "+++" are not skipped.
- Around line 829-849: The current addSourceOfTruthFindings function can drop
newly injected source-of-truth findings because it slices the combined array to
50 at the end; change the logic so the 50-item cap never truncates injected
findings: build the list by taking up to 50 items from the original findings
while reserving slots for any source-of-truth items you will add (or,
alternatively, append all source-of-truth items and if the combined length
exceeds 50, trim only from the original findings portion). Update
addSourceOfTruthFindings (and the augmented variable/return behavior) so that
any finding created for a review with status "missing" or "needs_followup" is
always included in the final array even when the total would exceed 50.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1df41838-e88c-4bbb-8acb-f368d8ed02b4

📥 Commits

Reviewing files that changed from the base of the PR and between 8be9986 and 0ffcd62.

📒 Files selected for processing (4)

• test/pr-review-advisor.test.ts
• tools/pr-review-advisor/README.md
• tools/pr-review-advisor/analyze.mts
• tools/pr-review-advisor/schema.json