Skip to content

fix(inference): use model-aware token field in rebuild preflight - #6860

Merged
cv merged 2 commits into
mainfrom
fix/preflight-gpt5-reply-budget
Jul 14, 2026
Merged

fix(inference): use model-aware token field in rebuild preflight#6860
cv merged 2 commits into
mainfrom
fix/preflight-gpt5-reply-budget

Conversation

@laitingsheng

@laitingsheng laitingsheng commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

The atomic DCode rebuild gate probes the sandbox's recorded OpenAI-compatible route before making changes. Its Chat Completions payload hard-coded max_tokens, so rebuilds configured with GPT-5 or o-series models could fail with HTTP 400 before rebuild processing began.

This PR makes that rebuild-only probe use the shared model-aware token-field resolver. It does not change the managed gateway's ongoing route-health validation.

Scope

preflightRebuildInferenceRoute is called only by the DCode rebuild preflight. This PR intentionally does not change the managed gateway's ongoing route-health validation or claim broader runtime behavior.

Changes

  • Resolve the Chat Completions reply-budget field with resolveMaxTokensField(input.model).
  • Send max_completion_tokens for GPT-5 and o1/o3/o4 model families.
  • Preserve max_tokens for models that support the legacy field.
  • Cover GPT-5, o-series, and legacy-model behavior at the rebuild-preflight command boundary.

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: internal rebuild inference preflight behavior; no user-facing contract changed.
  • 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:
  • Non-success, skipped, or missing CI check accepted by maintainer - check name, approval link, and follow-up issue:

Verification

  • PR description includes Signed-off-by: lines and every commit is signed
  • npm run check:diff
  • npm run build:cli
  • npm run typecheck:cli
  • npx vitest run --project cli src/lib/actions/sandbox/rebuild-inference-preflight.test.ts - 7 passed
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed

Signed-off-by: Tinson Lai tinsonl@nvidia.com
Signed-off-by: Prekshi Vyas prekshiv@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved inference preflight checks for newer GPT-5 and o-series models by using the correct completion token parameter.
    • Preserved compatibility with legacy models by continuing to use their supported token limit parameter.
    • Added coverage to verify request payloads use the appropriate field for each model type.

…PT-5

The chat completions branch hardcoded max_tokens, which GPT-5 and o-series reject (HTTP 400).

Route the reply-budget field through resolveMaxTokensField to match the fixed request path.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The inference preflight probe now selects the max-token field based on the model. Tests verify GPT-5 and o-series models use max_completion_tokens, while legacy models use max_tokens.

Changes

Inference preflight token selection

Layer / File(s) Summary
Model-aware probe payload and coverage
src/lib/actions/sandbox/rebuild-inference-preflight.ts, src/lib/actions/sandbox/rebuild-inference-preflight.test.ts
The probe payload resolves the max-token field from the model, with tests covering GPT-5, o-series, and legacy chat-completions requests.

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

Suggested labels: provider: nvidia

Suggested reviewers: cv, ericksoa, cjagwani

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 summarizes the main change: using a model-aware token field in the rebuild preflight probe.
✨ 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 fix/preflight-gpt5-reply-budget

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

@github-code-quality

github-code-quality Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the main branch.

TypeScript / code-coverage/cli

The overall coverage in the fix/preflight-gpt5-r... branch remains at 79%, unchanged from the main branch.

Show a code coverage summary of the most impacted files.
File main 20130dd fix/preflight-gpt5-r... 7b7640f +/-
src/lib/agent/dashboard-ui.ts 89% 83% -6%
src/lib/state/config-io.ts 94% 91% -3%
src/lib/agent/m...fest-readers.ts 97% 95% -2%
src/lib/state/gateway.ts 93% 91% -2%
src/lib/messagi...annel-config.ts 99% 98% -1%
src/lib/state/sandbox.ts 85% 85% 0%
src/lib/messagi.../persistence.ts 86% 89% +3%
src/lib/messagi...n-validation.ts 96% 100% +4%
src/lib/sandbox...rce-identity.ts 89% 94% +5%
src/lib/adapter...shell/client.ts 83% 88% +5%

Updated July 14, 2026 17:28 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@laitingsheng laitingsheng added NV QA Bugs found by the NVIDIA QA Team area: inference Inference routing, serving, model selection, or outputs bug-fix PR fixes a bug or regression labels Jul 14, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/lib/actions/sandbox/rebuild-inference-preflight.test.ts (1)

67-80: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Cover the o-series branch as well.

The resolver also maps o1/o3/o4 models to max_completion_tokens, but these action-level tests cover only GPT-5 and the fallback branch. Add an o-series case to protect the full model-selection contract.

Suggested test
+  it("sends max_completion_tokens for an o-series model on the chat completions route (`#6850`)", () => {
+    const command = buildRebuildInferenceProbeCommand({ ...input, model: "o3-mini" });
+
+    expect(command).toContain('"max_completion_tokens":8');
+    expect(command).not.toContain('"max_tokens"');
+  });
🤖 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 `@src/lib/actions/sandbox/rebuild-inference-preflight.test.ts` around lines 67
- 80, Add an action-level test alongside the existing GPT-5 and non-GPT-5 cases
in the resolver test suite, using an o-series model such as o1, o3, or o4.
Verify the generated chat-completions command contains max_completion_tokens
with the expected value and excludes max_tokens, covering the resolver’s
o-series branch.
🤖 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 `@src/lib/actions/sandbox/rebuild-inference-preflight.test.ts`:
- Around line 67-80: Add an action-level test alongside the existing GPT-5 and
non-GPT-5 cases in the resolver test suite, using an o-series model such as o1,
o3, or o4. Verify the generated chat-completions command contains
max_completion_tokens with the expected value and excludes max_tokens, covering
the resolver’s o-series branch.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 335b4448-bbdd-4793-bbe8-97c1ad0b04e5

📥 Commits

Reviewing files that changed from the base of the PR and between 33d55b0 and fa5dcdc.

📒 Files selected for processing (2)
  • src/lib/actions/sandbox/rebuild-inference-preflight.test.ts
  • src/lib/actions/sandbox/rebuild-inference-preflight.ts

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / high confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: inference-routing, network-policy, onboard-repair, onboard-resume, state-backup-restore, upgrade-stale-sandbox

1 optional E2E recommendation
  • sandbox-rebuild

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
@prekshivyas prekshivyas changed the title fix(inference): send max_completion_tokens in rebuild preflight for GPT-5 fix(inference): use model-aware token field in rebuild preflight Jul 14, 2026
@prekshivyas
prekshivyas requested a review from cv July 14, 2026 17:38
@cv
cv merged commit 0879587 into main Jul 14, 2026
92 of 109 checks passed
@cv
cv deleted the fix/preflight-gpt5-reply-budget branch July 14, 2026 17:46
jyaunches added a commit that referenced this pull request Jul 14, 2026
## Summary

Add v0.0.83 release notes to `docs/about/release-notes.mdx` for pre-tag
release prep.

## Source Summary

- #6773 -> `docs/about/release-notes.mdx`: Shared inference route
changes are explicit and fail-safe; status shows recorded route, live
route, and drift.
- #6875 -> `docs/about/release-notes.mdx`: DGX Station GB300 express
setup restored; vLLM storage preflight narrowed.
- #6770 -> `docs/about/release-notes.mdx`: Risky Spark vLLM server
warning during onboarding.
- #6856 -> `docs/about/release-notes.mdx`: Re-onboard reuse preserves
tier-default brave/tavily presets.
- #6867 -> `docs/about/release-notes.mdx`: Unreachable custom endpoint
routed through transport-recovery path.
- #6860 -> `docs/about/release-notes.mdx`: Rebuild preflight uses
model-aware token field for o-series/GPT-5.
- #6845 -> `docs/about/release-notes.mdx`: Corporate CA anchored for
image build TLS.
- #6833 -> `docs/about/release-notes.mdx`: SSH ControlMaster-delegated
forwards recognized in fallback.
- #6837 -> `docs/about/release-notes.mdx`: Hermes light skin writes via
stdin on macOS.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] 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:
- [x] Tests not applicable — justification: doc-only release notes
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed
- [ ] Non-success, skipped, or missing CI check accepted by maintainer

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as Verified in GitHub
- [x] Normal pre-commit, commit-msg, and pre-push hooks passed
- [x] `npm run docs` passes with 0 errors

Signed-off-by: Jessica Yaunches <jyaunches@nvidia.com>

Signed-off-by: Jessica Yaunches <jyaunches@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: inference Inference routing, serving, model selection, or outputs bug-fix PR fixes a bug or regression NV QA Bugs found by the NVIDIA QA Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants