Skip to content

[codex] fix compatible endpoint reasoning mode#5531

Open
HOYALIM wants to merge 3 commits into
NVIDIA:mainfrom
HOYALIM:codex/issue-3279-reasoning-option3
Open

[codex] fix compatible endpoint reasoning mode#5531
HOYALIM wants to merge 3 commits into
NVIDIA:mainfrom
HOYALIM:codex/issue-3279-reasoning-option3

Conversation

@HOYALIM

@HOYALIM HOYALIM commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds Option 3 OpenAI-compatible reasoning-mode configuration through NEMOCLAW_REASONING.
  • Prompts interactive custom endpoint users before sandbox creation and validates existing non-interactive env overrides.
  • Makes compatible-endpoint sandbox smoke failures actionable when the response has non-empty reasoning_content but empty choices[0].message.content.
  • Adds focused tests for env normalization, interactive selection, and reasoning-only smoke responses.

Validation

  • NODE_PATH=/Users/holim/code/NemoClaw/node_modules /Users/holim/code/NemoClaw/node_modules/.bin/vitest run src/lib/onboard/reasoning-mode.test.ts src/lib/onboard/compatible-endpoint-smoke.test.ts test/onboard-compatible-reasoning-mode.test.ts
  • NODE_PATH=/Users/holim/code/NemoClaw/node_modules /Users/holim/code/NemoClaw/node_modules/.bin/tsc -p tsconfig.src.json --noEmit
  • git diff --check

Fixes #3279

Summary by CodeRabbit

  • New Features

    • Added onboarding support for “reasoning mode” for OpenAI-compatible endpoints, including an interactive option that persists NEMOCLAW_REASONING as "true"/"false", with input normalization and clear prompts.
  • Bug Fixes

    • Enhanced compatible-endpoint smoke checks to fail fast when responses contain only reasoning (empty message content), with guidance to enable reasoning mode (or set NEMOCLAW_REASONING=true).
  • Tests

    • Expanded smoke and onboarding coverage for reasoning-mode behavior, including POSIX OpenAI-compatible integration tests plus unit checks for normalization and invalid env handling.

@copy-pr-bot

copy-pr-bot Bot commented Jun 17, 2026

Copy link
Copy Markdown

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

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ee59d4d2-89c0-40a7-9a39-e87f4522b911

📥 Commits

Reviewing files that changed from the base of the PR and between 31dfdb2 and 7be6ebf.

📒 Files selected for processing (6)
  • src/lib/onboard.ts
  • src/lib/onboard/compatible-endpoint-smoke.test.ts
  • src/lib/onboard/compatible-endpoint-smoke.ts
  • src/lib/onboard/reasoning-mode.test.ts
  • src/lib/onboard/reasoning-mode.ts
  • test/onboard-compatible-reasoning-mode.test.ts

📝 Walkthrough

Walkthrough

A new reasoning-mode.ts module is added to configure NEMOCLAW_REASONING for custom OpenAI-compatible endpoints. The onboarding handleRemoteProviderSelection path now calls configureCustomOpenAiReasoningModeOrExit when the "custom" provider is selected, before model validation. The smoke script gains a branch to detect reasoning-only responses (non-empty reasoning_content with empty content). Unit and integration tests cover all new paths.

Changes

Reasoning Mode for Custom OpenAI-Compatible Endpoints

Layer / File(s) Summary
ReasoningModeDeps contract and configuration logic
src/lib/onboard/reasoning-mode.ts
Exports ReasoningModeDeps type, ReasoningModeExitDeps contract, normalizeReasoningMode (trim/lowercase validation), configureCustomOpenAiReasoningMode (reads/validates/prompts/writes NEMOCLAW_REASONING), and configureCustomOpenAiReasoningModeOrExit (try/catch wrapper that exits with code 1 on error).
Custom provider onboarding wiring
src/lib/onboard.ts
In handleRemoteProviderSelection, inserts a call to configureCustomOpenAiReasoningModeOrExit before validateSelectedRemoteModel when the selected provider key is "custom". Removes the separate build-mode validation block and consolidates finalization. Updates documentation comments and credential-env hydration anchoring within the model-selection and remote-provider detection flow.
Smoke script reasoning-only response detection
src/lib/onboard/compatible-endpoint-smoke.ts
Adds a check_response branch in the embedded Python smoke script that detects non-empty reasoning_content with empty content, prints a diagnostic pointing to NEMOCLAW_REASONING=true, and calls sys.exit(1).
Unit tests: reasoning-mode module and smoke script
src/lib/onboard/reasoning-mode.test.ts, src/lib/onboard/compatible-endpoint-smoke.test.ts
Unit tests for normalizeReasoningMode normalization/validation, configureCustomOpenAiReasoningMode (non-interactive reuse, interactive prompt, invalid-value rejection), and the smoke script's reasoning-only failure path with stub curl.
Integration test: full onboarding with reasoning-only curl stub
test/onboard-compatible-reasoning-mode.test.ts
Spawns a generated Node script against a fake curl binary that returns HTTP 400 unless NEMOCLAW_REASONING=true; asserts successful onboarding exit, correct provider/model/preferred inference API, reasoning enabled to "true", and expected reasoning-mode enablement prompt output.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#5420: Modifies the same handleRemoteProviderSelection custom provider path around validateSelectedRemoteModel, directly adjacent to where this PR inserts the reasoning-mode configuration call.
  • NVIDIA/NemoClaw#5454: Changes the same selected.key === "build" and setupNim inference handling region in the provider-selection/validation flow that this PR refactors.

Suggested labels

area: onboarding

Suggested reviewers

  • cv

Poem

🐰 A reasoning model came knocking one day,
Its content was empty—no words left to say!
So I sniffed reasoning_content instead,
And set NEMOCLAW_REASONING before you were misled.
Now Option 3 knows just what to do—hooray! 🥕

🚥 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 '[codex] fix compatible endpoint reasoning mode' clearly and concisely summarizes the main change: adding reasoning mode configuration support for Option 3 compatible endpoints.
Linked Issues check ✅ Passed The PR fully addresses issue #3279 by implementing runtime NEMOCLAW_REASONING configuration during Option 3 onboarding and gracefully handling reasoning-only model responses with non-empty reasoning_content but empty content fields.
Out of Scope Changes check ✅ Passed All changes are directly scoped to resolving issue #3279: reasoning mode configuration in onboard.ts, reasoning-mode utility module, smoke test enhancements for reasoning-only responses, and comprehensive test coverage.
Docstring Coverage ✅ Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@HOYALIM HOYALIM force-pushed the codex/issue-3279-reasoning-option3 branch 2 times, most recently from f300eaf to b99fee0 Compare June 17, 2026 06:41
@HOYALIM HOYALIM marked this pull request as ready for review June 17, 2026 06:42
Copilot AI review requested due to automatic review settings June 17, 2026 06:42

Copilot AI 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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds onboarding support for enabling “reasoning mode” when using a custom OpenAI-compatible endpoint (Option 3), and improves smoke-test diagnostics for reasoning-only responses.

Changes:

  • Introduces a reasoning-mode onboarding helper that normalizes/validates NEMOCLAW_REASONING and prompts users when unset.
  • Updates the compatible-endpoint smoke check to detect “reasoning-only” responses and provide actionable guidance.
  • Adds unit + integration-style tests covering the new onboarding path and the updated smoke behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
test/onboard-compatible-reasoning-mode.test.ts Integration-style test that simulates Option 3 onboarding and verifies NEMOCLAW_REASONING is set.
src/lib/onboard/reasoning-mode.ts New helper for prompting/validating NEMOCLAW_REASONING and an “orExit” wrapper.
src/lib/onboard/reasoning-mode.test.ts Unit tests for env normalization, prompting, and validation error behavior.
src/lib/onboard/compatible-endpoint-smoke.ts Enhances smoke-check failure mode messaging for reasoning-only outputs.
src/lib/onboard/compatible-endpoint-smoke.test.ts Adds regression test for reasoning-only response diagnostics.
src/lib/onboard.ts Hooks Option 3 (“custom”) selection into the new reasoning-mode configuration step.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/lib/onboard/reasoning-mode.ts
Comment thread src/lib/onboard/reasoning-mode.ts
Comment thread src/lib/onboard.ts Outdated
return "retry-selection";
}

if (selected.key === "custom") await require("./onboard/reasoning-mode").configureCustomOpenAiReasoningModeOrExit({ isNonInteractive, promptYesNoOrDefault, note });
Comment thread src/lib/onboard/reasoning-mode.test.ts Outdated
);
});

it("prompts Option 3 users and writes the Dockerfile ARG source env", async () => {
Comment thread test/onboard-compatible-reasoning-mode.test.ts
Comment thread test/onboard-compatible-reasoning-mode.test.ts
env: {
...process.env,
HOME: tmpDir,
PATH: `${fakeBin}:${process.env.PATH || ""}`,
@HOYALIM HOYALIM force-pushed the codex/issue-3279-reasoning-option3 branch from b99fee0 to 9104d05 Compare June 17, 2026 06:54
@HOYALIM HOYALIM force-pushed the codex/issue-3279-reasoning-option3 branch from cd0f350 to 7be6ebf Compare June 18, 2026 01:17
@HOYALIM

HOYALIM commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@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/onboard.ts (1)

3911-3920: Run the onboarding E2E bundle for this flow change.

This wiring sits on the custom provider’s core selection/validation path, so it’s worth running the listed E2E jobs before merge to catch regressions across sandbox lifecycle and compatible-endpoint behavior.

As per coding guidelines, changes in src/lib/onboard.ts should be validated with the recommended E2E workflow job set (including cloud-e2e, sandbox-operations-e2e, rebuild-openclaw-e2e, channels-stop-start-e2e, messaging-compatible-endpoint-e2e, bedrock-runtime-compatible-anthropic-e2e, hermes-discord-e2e, hermes-slack-e2e, openshell-gateway-upgrade-e2e, and issue-3600-gpu-proof-optional-e2e).

🤖 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/onboard.ts` around lines 3911 - 3920, Before merging your changes to
the custom provider's core selection/validation path in the
reasoningMode.configureCustomOpenAiReasoningModeOrExit call, you must run the
full recommended E2E test workflow to catch any regressions. Execute the
following E2E jobs: cloud-e2e, sandbox-operations-e2e, rebuild-openclaw-e2e,
channels-stop-start-e2e, messaging-compatible-endpoint-e2e,
bedrock-runtime-compatible-anthropic-e2e, hermes-discord-e2e, hermes-slack-e2e,
openshell-gateway-upgrade-e2e, and issue-3600-gpu-proof-optional-e2e to validate
the sandbox lifecycle and compatible-endpoint behavior remain intact after your
modifications to src/lib/onboard.ts.

Source: Coding guidelines

🤖 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/onboard.ts`:
- Around line 3911-3920: Before merging your changes to the custom provider's
core selection/validation path in the
reasoningMode.configureCustomOpenAiReasoningModeOrExit call, you must run the
full recommended E2E test workflow to catch any regressions. Execute the
following E2E jobs: cloud-e2e, sandbox-operations-e2e, rebuild-openclaw-e2e,
channels-stop-start-e2e, messaging-compatible-endpoint-e2e,
bedrock-runtime-compatible-anthropic-e2e, hermes-discord-e2e, hermes-slack-e2e,
openshell-gateway-upgrade-e2e, and issue-3600-gpu-proof-optional-e2e to validate
the sandbox lifecycle and compatible-endpoint behavior remain intact after your
modifications to src/lib/onboard.ts.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0f661b9f-6816-4d8b-b97b-f9353a68d7f6

📥 Commits

Reviewing files that changed from the base of the PR and between 31dfdb2 and 7be6ebf.

📒 Files selected for processing (6)
  • src/lib/onboard.ts
  • src/lib/onboard/compatible-endpoint-smoke.test.ts
  • src/lib/onboard/compatible-endpoint-smoke.ts
  • src/lib/onboard/reasoning-mode.test.ts
  • src/lib/onboard/reasoning-mode.ts
  • test/onboard-compatible-reasoning-mode.test.ts

@HOYALIM

HOYALIM commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai summary

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Summary regeneration triggered.

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.

NEMOCLAW_REASONING not configurable for Option 3 providers; reasoning-only models fail silently

2 participants