Skip to content

fix(review): correct OpenAI advisor model ID - #10835

Merged
cv merged 1 commit into
mainfrom
fix/advisor-openai-model-id
Sep 2, 2026
Merged

cv merged 1 commit into
mainfrom
fix/advisor-openai-model-id

Conversation

@cv

@cv cv commented Sep 2, 2026 •

Copy link
Copy Markdown
Collaborator

Outcome

OpenAI-routed PR Review Advisor specialists configure successfully while the inference service continues to receive openai/openai/gpt-5.6-terra.

Reason

The Pi registry prepends its local openai provider name to the configured model ID. Supplying openai/openai/gpt-5.6-terra therefore attempted to resolve openai/openai/openai/gpt-5.6-terra and caused half of each advisor matrix to fail before inference.

Changes

  • Configure the OpenAI matrix lane with openai/gpt-5.6-terra.
  • Update the matrix expectation to protect the corrected identifier.

Verification

  • Local validation intentionally skipped for this urgent two-line correction at maintainer direction.
  • Runtime evidence: advisor run 33576202233 showed all OpenAI-lane specialists failing to configure the triply prefixed identifier while Azure-lane specialists succeeded.
  • Secrets review: The diff contains no secrets, API keys, or credentials.

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

Summary by CodeRabbit

  • Bug Fixes
    • Corrected the default OpenAI model configuration to use the proper identifier.
    • Updated workflow validation to reflect the corrected model setting.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv self-assigned this Sep 2, 2026
@coderabbitai

coderabbitai Bot commented Sep 2, 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: 8730b8a6-4b3b-40f1-9905-2436ef12a3ac

📥 Commits

Reviewing files that changed from the base of the PR and between 95c0a60 and 37d4e8e.

📒 Files selected for processing (2)
  • test/automation/pull-requests/pr-review-advisor-specialists.test.ts
  • tools/pr-review-advisor/render-specialist-matrix.mts

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The default specialist matrix and its test now use openai/gpt-5.6-terra instead of the duplicated openai/openai/ prefix. The Azure fallback is unchanged.

Changes

Model identifier correction

Layer / File(s) Summary
Correct the specialist model identifier
tools/pr-review-advisor/render-specialist-matrix.mts, test/automation/pull-requests/pr-review-advisor-specialists.test.ts
The default model and workflow-matrix expectation now use openai/gpt-5.6-terra. The Azure identifier remains unchanged.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 37d4e

The OpenAI advisor matrix now uses the corrected model identifier while preserving the Azure configuration, so no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: brandonpelfrey

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: correcting the OpenAI advisor model ID.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
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.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.

✨ 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 fix/advisor-openai-model-id

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

@github-code-quality

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit 37d4e8e in the fix/advisor-openai-m... branch remains at 96%, unchanged from commit 95c0a60 in the main branch.

@cv
cv merged commit 0353ebb into main Sep 2, 2026
72 of 77 checks passed
@cv
cv deleted the fix/advisor-openai-model-id branch September 2, 2026 01:01
cv added a commit that referenced this pull request Sep 2, 2026
<!-- markdownlint-disable MD041 -->
## Outcome

Both PR Review Advisor model lanes use their complete upstream model IDs
and configure successfully in the embedded advisor registry.

## Reason

OpenShell forwards the configured model ID verbatim and requires
`openai/openai/gpt-5.6-terra`. The embedded Pi registry previously
registered only the Azure default, so the complete OpenAI ID failed
local lookup. Shortening that ID passed local lookup only after changing
the matrix, but OpenShell then rejected it with HTTP 403 because the key
was not authorized for `openai/gpt-5.6-terra`.

## Changes

- Restore the complete `openai/openai/gpt-5.6-terra` matrix identifier.
- Register the selected runtime model ID in the embedded advisor
provider configuration.
- Pass the selected ID through advisor configuration setup.
- Cover selected-model registration and the restored matrix value.

## Verification

- Local validation intentionally skipped at maintainer direction for
this follow-up.
- Runtime evidence: post-#10835 run 33577784873 reached OpenShell with
the shortened ID and received `key_model_access_denied` for
`openai/gpt-5.6-terra`; Azure specialists succeeded.
- Secrets review: The diff contains no secrets, API keys, or
credentials.

---

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

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@wscurran wscurran added the chore Build, CI, dependency, or tooling maintenance label Sep 9, 2026
@wscurran wscurran added the area: ci CI workflows, checks, release automation, or GitHub Actions label Oct 31, 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 chore Build, CI, dependency, or tooling maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants