Skip to content

fix(ci): pass comparison refs to advisor preparation - #10147

Closed
apurvvkumaria wants to merge 1 commit into
mainfrom
codex/fix-advisor-prepare-refs
Closed

fix(ci): pass comparison refs to advisor preparation#10147
apurvvkumaria wants to merge 1 commit into
mainfrom
codex/fix-advisor-prepare-refs

Conversation

@apurvvkumaria

@apurvvkumaria apurvvkumaria commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

The advisor now supplies its selected base and head refs while preparing specialist context. Without them, every specialist stops before analysis and no review artifact is produced.

This completes the specialist-context wiring introduced in #10075 and restores automated review for the current merge train.

Related issue

Regression from #10075.

Changes

  • Pass the selected comparison refs to the trusted preparation step.
  • Extend the workflow-boundary validator to require both inputs.
  • Add a mutation test that removes the inputs and proves validation fails.

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:
  • 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: Maintainer review confirms the new values are repository-owned expressions passed only to trusted preparation code. PR content remains read-only data, the GitHub token remains confined to the preparation step, and model credentials are unchanged.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: Not applicable; no DGX Station host preparation changes.
  • Station profile/scenario:
  • Result:
  • Supporting evidence:

Verification

  • PR description includes a Signed-off-by line and every published commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed
  • Workflow-boundary test passes: 9 tests
  • Repository checks pass
  • JavaScript configuration type-checking passes after building the CLI
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • 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: Apurv Kumaria akumaria@nvidia.com

Summary by CodeRabbit

  • Bug Fixes

    • Improved pull request review analyses by consistently selecting the correct base and head revisions.
    • Added validation to detect missing revision references before specialist analysis begins.
  • Tests

    • Added workflow coverage for validating required base and head revision inputs.

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

This repository limits you to 10 open pull requests. Please close or merge an existing PR before opening another one.

@github-actions github-actions Bot closed this Aug 24, 2026
@github-code-quality

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit d32f705 in the codex/fix-advisor-pr... branch remains at 96%, unchanged from commit 009ab57 in the main branch.

@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b7bcb6f0-20a8-44fa-b8fa-f1e19a0246be

📥 Commits

Reviewing files that changed from the base of the PR and between 009ab57 and d32f705.

📒 Files selected for processing (3)
  • .github/workflows/pr-review-advisor.yaml
  • test/pr-review-advisor-workflow-boundary.test.ts
  • tools/pr-review-advisor/workflow-boundary.mts

📝 Walkthrough

Walkthrough

The advisor workflow now selects base and head refs for sandbox preparation. Boundary validation checks that both environment variables receive the expected values and reports missing refs separately.

Changes

Advisor ref propagation

Layer / File(s) Summary
Ref selection and workflow wiring
.github/workflows/pr-review-advisor.yaml, tools/pr-review-advisor/workflow-boundary.mts
The workflow selects refs for pull requests, targeted manual analyses, and other workflow inputs. It passes the selected refs to sandbox preparation.
Boundary validation
tools/pr-review-advisor/workflow-boundary.mts, test/pr-review-advisor-workflow-boundary.test.ts
Validation and tests check both selected ref variables and report missing base or head refs separately.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: brandonpelfrey

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-advisor-prepare-refs

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

@apurvvkumaria

Copy link
Copy Markdown
Collaborator Author

Blocked by the repository's open-PR limit.

The workflow repair is implemented and locally verified: the focused boundary suite passes 9 tests, repository checks pass, the CLI build and type check pass, normal hooks pass, the commit is signed and Verified, and the change is 28 additions across 3 files.

The repository closed this PR because my account had 11 open PRs against a limit of 10. I will not reopen it until a slot is available and will not bypass the policy. Until then, fresh PR Review Advisor runs using the affected workflow revision can fail before analysis because the sandbox preparation step does not receive the selected comparison refs.

Carlos Villela's specialist-context work in #10075 remains credited as the original implementation. This PR is a narrow follow-up for the workflow wiring regression.

cv pushed a commit that referenced this pull request Aug 24, 2026
## Summary

The advisor now supplies its selected base and head refs while preparing
specialist context. Without them, every specialist stops before analysis
and no review artifact is produced.

This preserves the repair from #10147, which the repository closed only
because its author reached the open-PR limit, and refreshes it onto
current `main` without conflicts.

## Related Issue

Regression from #10075. Replaces #10147.

## Changes

- Pass the selected comparison refs to the trusted preparation step.
- Extend the workflow-boundary validator to require both inputs.
- Add a mutation test that removes the inputs and proves validation
fails.

## Type of Change

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

- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: The values are
repository-owned expressions passed only to trusted preparation code. PR
content remains read-only data, the GitHub token remains confined to the
preparation step, and model credentials are unchanged.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit: Not applicable; no DGX Station host preparation
changes.
- Station profile/scenario:
- Result:
- Supporting evidence:

## Verification

- [x] PR description includes a `Signed-off-by:` line and every
published commit appears as `Verified` in GitHub
- [x] Normal pre-commit hooks passed on the refreshed exact diff
- [x] Targeted workflow-boundary test passes: 9 tests
- [x] Repository checks pass
- [x] CLI build and JavaScript configuration type-checking pass
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [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)
- [ ] Doc pages follow the style guide (doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Bug Fixes**
- Improved pull request review analysis to consistently compare the
correct base and head revisions across supported workflow types.
- Added validation to detect missing revision references before
specialist analysis begins, providing clearer errors and preventing
incomplete comparisons.

- **Tests**
- Expanded workflow boundary coverage to verify base and head revision
inputs independently across supported analysis scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Apurv Kumaria <akumaria@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant