Skip to content

feat(advisor): enrich findings and retry synthesis - #5572

Merged
cv merged 11 commits into
mainfrom
feat/pr-review-advisor-adversarial-context
Jun 21, 2026
Merged

feat(advisor): enrich findings and retry synthesis#5572
cv merged 11 commits into
mainfrom
feat/pr-review-advisor-adversarial-context

Conversation

@cv

@cv cv commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds adversarial-review structure to the PR Review Advisor so findings are easier to verify and follow up. The advisor now records hidden sticky-comment metadata, emits probe-shaped finding fields, retries low-quality synthesis once, writes auditable deterministic context artifacts, and uses static test inventory to make coverage guidance less generic.

Changes

  • Add hidden head-SHA/run metadata and review posture to PR Review Advisor sticky comments.
  • Extend finding schema/rendering with impact, verification hints, and missing regression-test guidance.
  • Add retry synthesis when the first advisor output is malformed or contains placeholder structured fields.
  • Add static test inventory context from changed test files and nearby test names.
  • Write deterministic drift/security/validation context artifacts plus PR diff and prior advisor comments.
  • Update PR Review Advisor tests and README for the new output contract and artifacts.

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

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior
  • Full npm test passes (broad runtime changes only)
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • 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: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

Release Notes

  • New Features
    • Findings now require probe-shaped Impact, Verification hint, and Missing regression test; advisor comments render these fields and include a computed Review posture.
    • Deterministic review context now writes validation-context.json with a static test-inventory to support reliable retries.
    • Improved retry-synthesis for malformed/low-quality outputs, including retry-reason logging and first-pass preservation.
  • Documentation
    • Updated the PR Review Advisor README for enhanced prior-follow-up handling and retry artifacts.
  • Tests
    • Expanded coverage for deterministic artifacts, trusted prior-advisor handling, sticky-comment rendering, and retry/quality behavior.

@cv cv self-assigned this Jun 21, 2026
@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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: 1e42212b-701c-4899-ba8e-4f13fc6fae08

📥 Commits

Reviewing files that changed from the base of the PR and between c6b0928 and eab914e.

📒 Files selected for processing (2)
  • test/pr-review-advisor.test.ts
  • tools/pr-review-advisor/comment.mts
🚧 Files skipped from review as they are similar to previous changes (2)
  • tools/pr-review-advisor/comment.mts
  • test/pr-review-advisor.test.ts

📝 Walkthrough

Walkthrough

The PR enriches the PR review advisor's finding schema with three new required fields (impact, verificationHint, missingRegressionTest), adds static regex-based test inventory collection from changed test files, introduces a quality-check-driven retry synthesis path, writes deterministic context artifacts to disk, embeds hidden run metadata in sticky comments, and extends tests and README documentation to cover all new behaviors.

Changes

PR Review Advisor: enriched findings, retry synthesis, static test inventory

Layer / File(s) Summary
Finding schema and type contract
tools/pr-review-advisor/schema.json, tools/pr-review-advisor/analyze.mts, tools/pr-review-advisor/comment.mts
impact, verificationHint, and missingRegressionTest are added to $defs.finding.required in the JSON schema; types Finding, StaticTestInventory, DeterministicReviewContext, PreviousAdvisorReview, and ReviewMetadata are exported in analyze.mts; and finding shape in comment.mts is updated to include optional enriched fields.
Static test inventory and deterministic context artifacts
tools/pr-review-advisor/analyze.mts
New collectStaticTestInventory() function regex-extracts describe/it/test names from changed test files with bounded reads and symlink skipping. The inventory is wired into collectDeterministicContext(), included in buildValidationTurnContext() for turn-3 validation, and persisted via writeDeterministicContextArtifacts() which writes JSON, diff, and optional prior-review artifacts to deterministic context directory.
Advisor execution, parsing, quality checks, and retry orchestration
tools/pr-review-advisor/analyze.mts
ArtifactPaths gains context and retry artifact paths. runAdvisorConversation() wraps SDK execution and throws on turnErrors. parseAdvisorResult() normalizes extracted JSON. reviewQualityIssues() detects placeholder/missing enriched fields. When issues detected, buildRetryPromptTurns() constructs targeted repair prompt; conversation reruns once. Retry failure calls recordRetryFailureOnFirstPass() to preserve first-pass results with warning annotation.
Previous advisor trust validation and metadata extraction
tools/pr-review-advisor/analyze.mts
extractPreviousAdvisorReview() parses trusted advisor hidden HTML markers (head_sha:, run_id:, run_attempt:, comment_id:), validates prior comment provenance via GitHub Actions API check, filters spoofed/untrusted/collision/replay scenarios, and integrates trusted prior-review context. Integration point collectTrustedPreviousAdvisorReview() wires trusted extraction into collectGitHubContext().
Prompt engineering and finding normalization
tools/pr-review-advisor/analyze.mts
System prompt requires every finding to be "probe-shaped" with concrete enriched fields. Turn-3 includes staticTestInventory in validation context. Turn-4 synthesis text constrains non-placeholder impact, verificationHint, missingRegressionTest and suggests timing for in-scope improvements. buildRetryPromptTurns() injects retry reasoning and prior output. sanitizeFindings() populates enriched defaults. Source-of-truth injection and appendFindings() fallback finding include enriched metadata.
Comment rendering with hidden metadata and enriched findings
tools/pr-review-advisor/comment.mts, tools/advisors/github.mts
CommentMetadata type carries runId/runAttempt/optional commentId from GitHub env. buildComment() accepts optional metadata, renders sanitized hidden HTML block via renderHiddenMetadata(), computes "Review posture" via reviewPosture(). Testing follow-up generation is refactored into collectTestingFollowups(), formatTestingFollowup(), and uniqueTestingFollowups() with deduplication and truncation. formatFinding() emits indented bullets for Impact, Verification hint, Missing regression test. upsertStickyComment() gains optional bodyForComment() callback to compute post-creation body from the created/updated comment object.
Test fixtures, imports, and supporting setup
test/pr-review-advisor.test.ts
Updated module imports to include new helpers: collectStaticTestInventory, buildRetryPromptTurns, writeDeterministicContextArtifacts, extractPreviousAdvisorReview, collectTrustedPreviousAdvisorReview, reviewQualityIssues, recordRetryFailureOnFirstPass, retryReasonLogSummary, upsertStickyComment. Extended metadata() fixture with staticTestInventory. Enhanced mocked findings in validResult() and inScopeFinding() with enriched fields.
Test cases for new features and updated assertions
test/pr-review-advisor.test.ts
Added tests: static inventory collection (symlink skipping), retry synthesis prompt construction, deterministic artifact writing with validation context, previous-advisor extraction with trusted metadata parsing and API validation, quality-issue detection and retry/failure workflows, sticky-comment upsert behavior, and test-followup markdown escaping. Updated system-prompt assertion to enforce "probe-shaped" findings. Updated acceptance-turn assertion to include staticTestInventory. Updated comment-rendering assertions to include hidden head_sha: marker and enriched finding fields. Tightened "Analyzed SHA" negative assertion.
README documentation
tools/pr-review-advisor/README.md
Expanded scope bullets to cover prior follow-up via sticky-comment metadata, static test-inventory context, codebase drift/architecture guardrails, and tolerant parsing/workaround behavior. Extended workflow steps to detail retry-on-quality-failure synthesis, artifact writing, and sticky comment posting with hidden run metadata. Added artifacts section documenting retry prompts, deterministic context JSON, diff file, prior-review context, and raw retry output. Refined output contract to specify enriched finding fields, probe-shaped requirements, and maintainer-review expectations for changed-code suggestions.

Sequence Diagram(s)

sequenceDiagram
  participant main as main()
  participant writeDCA as writeDeterministicContextArtifacts
  participant runAC as runAdvisorConversation
  participant parseAR as parseAdvisorResult
  participant rQI as reviewQualityIssues
  participant buildRT as buildRetryPromptTurns

  main->>writeDCA: pr.diff and deterministic context
  writeDCA-->>main: context artifacts written

  main->>runAC: initial advisor turns
  runAC-->>main: raw output (throws on turnErrors)

  main->>parseAR: normalize output
  parseAR-->>main: ReviewAdvisorResult | null

  main->>rQI: inspect for quality issues
  rQI-->>main: issues[]

  alt quality issues present
    main->>buildRT: reason + previous output + metadata
    buildRT-->>main: retry AdvisorPromptTurn[]
    main->>runAC: retry turn
    runAC-->>main: repaired output
    main->>parseAR: normalize repaired result
    parseAR-->>main: refined ReviewAdvisorResult
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#5170: Both PRs modify the PR Review Advisor's system-prompt/rubric and corresponding test assertions in tools/pr-review-advisor/analyze.mts, though with different new probe-shaped field requirements.
  • NVIDIA/NemoClaw#5386: This PR's staticTestInventory and synthetic tool-result injection extend the deterministic-context preloading refactor introduced in that PR.
  • NVIDIA/NemoClaw#5567: Both PRs modify the main advisor execution flow and finding-section rendering (enriched field display and suggestion posture guidance) in the comment and analyze modules.

Poem

🐇 Hop hop, the advisor grows strong!
Three enriched fields that right the wrong—
Impact, hints, and test gaps clear,
Retry when quality's ever near.
Hidden SHA marks each sage advice,
While the rabbit refines things twice! 🌟

🚥 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 'feat(advisor): enrich findings and retry synthesis' directly summarizes the main changes: extended finding schema with richer fields and implementation of retry synthesis logic for quality-gated output generation.
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 feat/pr-review-advisor-adversarial-context

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

@github-code-quality

github-code-quality Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the feat/pr-review-advis... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main feat/pr-review-advis... eab914e +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the feat/pr-review-advis... branch is 46%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main feat/pr-review-advis... eab914e +/-
src/lib/state/o...oard-session.ts 91%
src/lib/inference/local.ts 76%
src/lib/sandbox/config.ts 72%
src/lib/actions...dbox/rebuild.ts 67%
src/lib/onboard/preflight.ts 64%
src/lib/actions...licy-channel.ts 56%
src/lib/state/sandbox.ts 55%
src/lib/policy/index.ts 49%
src/lib/onboard...er-gpu-patch.ts 44%
src/lib/onboard.ts 18%

Updated June 21, 2026 22:24 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

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 runtime E2E is needed. The changes are confined to PR review advisor CI tooling, schema/comment rendering, trusted previous-comment handling, deterministic prompt artifacts, and a shared GitHub sticky-comment helper. They do not affect installer/onboarding, sandbox lifecycle, credentials used by the product, network policy, inference routing, deployment, or real assistant user flows; targeted unit tests were added/updated for the changed advisor behavior.

Optional E2E

  • None.

New E2E recommendations

  • None.

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: None
Optional Vitest E2E scenarios: None

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • None. Changes are limited to the PR Review Advisor implementation, its schema/README, and non-E2E unit tests. They do not touch test/e2e-scenario/, .github/workflows/e2e-vitest-scenarios.yaml, or other Vitest scenario dispatch machinery.

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • None.

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

Action expectation: Address required items before merge. Resolve or explicitly justify warnings. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up.
Findings: 0 required fixes, 3 items to resolve/justify, 2 in-scope improvements
Since last review: 0 prior items resolved, 2 still apply, 2 new items found

Review findings by urgency: 0 required fixes, 3 items to resolve/justify, 2 in-scope improvements

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

  • None.

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

  • Source-of-truth review needed: Previous advisor sticky-comment provenance: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
    • Evidence: README documents the residual same-run boundary, while collectTrustedPreviousAdvisorReview still trusts all candidates that match the same run window and extractPreviousAdvisorReview selects the last trusted candidate.
  • Same-run advisor comment replay boundary remains untested (tools/pr-review-advisor/analyze.mts:1159): collectTrustedPreviousAdvisorReview still validates each marker-bearing github-actions[bot] comment independently against run metadata and then extractPreviousAdvisorReview selects the last trusted candidate. A later bot-authored marker with copied run_id, head_sha, and run_attempt, its own matching comment_id, and updated_at inside the PR Review / Advisor run window can displace the original advisor context. The README and code now document this as a residual boundary, but the test suite does not cover the exact accepted replay shape.
    • Recommendation: Either change the provenance selection so later same-run marker comments cannot replace the original sticky advisor comment, or add an explicit mocked test for the documented residual boundary that proves the intended behavior for a later same-run github-actions[bot] marker with its own matching comment_id and in-window updated_at.
    • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
    • Evidence: collectTrustedPreviousAdvisorReview adds every candidate passing isTrustedAdvisorRun to trustedCommentIds, and extractPreviousAdvisorReview returns candidates.at(-1). Tests cover copied original comment_id, run-attempt mismatch, and out-of-window updates, but not the own-comment-id in-window replay shape described in the prior advisor finding.
  • Create-then-patch sticky comment can leave incomplete provenance metadata (tools/advisors/github.mts:130): When no sticky comment exists, upsertStickyComment first POSTs the placeholder body and then PATCHes the created comment with bodyForComment so the body can include the assigned comment id. If the PATCH fails after the POST succeeds, the PR can be left with a marker-bearing comment that lacks complete hidden provenance metadata until a later run repairs it.
    • Recommendation: Add a negative test and either tolerate the known incomplete-comment state explicitly or add a small recovery path, such as deleting the just-created placeholder on PATCH failure when permissions allow, retrying the PATCH, or ensuring future runs deterministically repair the marker-bearing comment.
    • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
    • Evidence: The new created-comment path POSTs body at tools/advisors/github.mts and only afterwards PATCHes repos/{repo}/issues/comments/{created.id}; tests cover successful create-and-patch and existing-comment patch paths, but not the POST-success/PATCH-failure branch.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

  • Advisor analyzer monolith absorbed several new responsibilities (tools/pr-review-advisor/analyze.mts:1): The PR adds retry orchestration, deterministic context artifacts, static test inventory, prior-review provenance validation, quality gating, prompt construction changes, and normalization/rendering support into analyze.mts. This keeps the behavior local, but the file is becoming harder to review for security-sensitive advisor boundaries.
    • Recommendation: Consider extracting cohesive helpers for provenance validation, retry synthesis, and static test inventory while this code is already being changed, or document why keeping them co-located is necessary for this PR's provenance/context boundary.
    • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
    • Evidence: The diff adds hundreds of lines to tools/pr-review-advisor/analyze.mts and the drift context shows this file is an active advisor hotspot with recent related changes.
  • Add exact same-run provenance replay test (test/pr-review-advisor.test.ts:668): The current provenance tests exercise many negative cases, but the behavior-specific case from the previous advisor comment remains uncovered: original trusted advisor comment plus later github-actions[bot] marker that copies run metadata, uses its own matching comment_id, and updates inside the same run window.
    • Recommendation: Add a mocked collectTrustedPreviousAdvisorReview test for that exact shape and assert either preservation of the original sticky comment or deliberate acceptance of the documented residual boundary.
    • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
    • Evidence: Existing tests include out-of-window replay and copied original comment_id cases, while the same-run own-comment-id replay remains absent from the visible test block.
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • **Mocked behavioral coverage** — collectTrustedPreviousAdvisorReview handles a later same-run github-actions[bot] marker with copied run metadata, its own matching comment_id, and updated_at inside the PR Review / Advisor run window according to the documented trust boundary.. Changed behavior centers on GitHub API provenance, sticky-comment upsert flows, filesystem reads, model-output validation, prompt/artifact construction, and comment rendering. The PR adds broad mocked coverage for these boundaries, with one behavior-specific provenance replay case still missing.
  • **Mocked behavioral coverage** — upsertStickyComment handles POST success followed by PATCH failure without leaving the advisor in an ambiguous sticky-comment provenance state.. Changed behavior centers on GitHub API provenance, sticky-comment upsert flows, filesystem reads, model-output validation, prompt/artifact construction, and comment rendering. The PR adds broad mocked coverage for these boundaries, with one behavior-specific provenance replay case still missing.
  • **Add exact same-run provenance replay test** — Add a mocked collectTrustedPreviousAdvisorReview test for that exact shape and assert either preservation of the original sticky comment or deliberate acceptance of the documented residual boundary.
  • **Previous advisor sticky-comment provenance** — Add a mocked collectTrustedPreviousAdvisorReview test where an original advisor comment is trusted and a later github-actions[bot] marker copies run_id, head_sha, and run_attempt, uses its own matching comment_id, has updated_at inside the same run window, and must either preserve the original or prove the documented boundary is intentionally accepted.. README documents the residual same-run boundary, while collectTrustedPreviousAdvisorReview still trusts all candidates that match the same run window and extractPreviousAdvisorReview selects the last trusted candidate.
Since last review details

Current findings, using the urgency labels above:

  • Source-of-truth review needed: Previous advisor sticky-comment provenance: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
    • Evidence: README documents the residual same-run boundary, while collectTrustedPreviousAdvisorReview still trusts all candidates that match the same run window and extractPreviousAdvisorReview selects the last trusted candidate.
  • Same-run advisor comment replay boundary remains untested (tools/pr-review-advisor/analyze.mts:1159): collectTrustedPreviousAdvisorReview still validates each marker-bearing github-actions[bot] comment independently against run metadata and then extractPreviousAdvisorReview selects the last trusted candidate. A later bot-authored marker with copied run_id, head_sha, and run_attempt, its own matching comment_id, and updated_at inside the PR Review / Advisor run window can displace the original advisor context. The README and code now document this as a residual boundary, but the test suite does not cover the exact accepted replay shape.
    • Recommendation: Either change the provenance selection so later same-run marker comments cannot replace the original sticky advisor comment, or add an explicit mocked test for the documented residual boundary that proves the intended behavior for a later same-run github-actions[bot] marker with its own matching comment_id and in-window updated_at.
    • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
    • Evidence: collectTrustedPreviousAdvisorReview adds every candidate passing isTrustedAdvisorRun to trustedCommentIds, and extractPreviousAdvisorReview returns candidates.at(-1). Tests cover copied original comment_id, run-attempt mismatch, and out-of-window updates, but not the own-comment-id in-window replay shape described in the prior advisor finding.
  • Create-then-patch sticky comment can leave incomplete provenance metadata (tools/advisors/github.mts:130): When no sticky comment exists, upsertStickyComment first POSTs the placeholder body and then PATCHes the created comment with bodyForComment so the body can include the assigned comment id. If the PATCH fails after the POST succeeds, the PR can be left with a marker-bearing comment that lacks complete hidden provenance metadata until a later run repairs it.
    • Recommendation: Add a negative test and either tolerate the known incomplete-comment state explicitly or add a small recovery path, such as deleting the just-created placeholder on PATCH failure when permissions allow, retrying the PATCH, or ensuring future runs deterministically repair the marker-bearing comment.
    • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
    • Evidence: The new created-comment path POSTs body at tools/advisors/github.mts and only afterwards PATCHes repos/{repo}/issues/comments/{created.id}; tests cover successful create-and-patch and existing-comment patch paths, but not the POST-success/PATCH-failure branch.
  • Advisor analyzer monolith absorbed several new responsibilities (tools/pr-review-advisor/analyze.mts:1): The PR adds retry orchestration, deterministic context artifacts, static test inventory, prior-review provenance validation, quality gating, prompt construction changes, and normalization/rendering support into analyze.mts. This keeps the behavior local, but the file is becoming harder to review for security-sensitive advisor boundaries.
    • Recommendation: Consider extracting cohesive helpers for provenance validation, retry synthesis, and static test inventory while this code is already being changed, or document why keeping them co-located is necessary for this PR's provenance/context boundary.
    • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
    • Evidence: The diff adds hundreds of lines to tools/pr-review-advisor/analyze.mts and the drift context shows this file is an active advisor hotspot with recent related changes.
  • Add exact same-run provenance replay test (test/pr-review-advisor.test.ts:668): The current provenance tests exercise many negative cases, but the behavior-specific case from the previous advisor comment remains uncovered: original trusted advisor comment plus later github-actions[bot] marker that copies run metadata, uses its own matching comment_id, and updates inside the same run window.
    • Recommendation: Add a mocked collectTrustedPreviousAdvisorReview test for that exact shape and assert either preservation of the original sticky comment or deliberate acceptance of the documented residual boundary.
    • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
    • Evidence: Existing tests include out-of-window replay and copied original comment_id cases, while the same-run own-comment-id replay remains absent from the visible test block.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each finding. A human maintainer must make the final merge decision.

@cv cv added the v0.0.66 label Jun 21, 2026
@cv cv changed the title feat(advisor): add adversarial review context feat(advisor): enrich findings and retry synthesis Jun 21, 2026
@cv
cv merged commit 79abc94 into main Jun 21, 2026
45 checks passed
@cv
cv deleted the feat/pr-review-advisor-adversarial-context branch June 21, 2026 23:03
@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions feature PR adds or expands user-visible functionality labels Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions feature PR adds or expands user-visible functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants