[codex] fix compatible endpoint reasoning mode#5531
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughA new ChangesReasoning Mode for Custom OpenAI-Compatible Endpoints
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
f300eaf to
b99fee0
Compare
There was a problem hiding this comment.
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_REASONINGand 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.
| return "retry-selection"; | ||
| } | ||
|
|
||
| if (selected.key === "custom") await require("./onboard/reasoning-mode").configureCustomOpenAiReasoningModeOrExit({ isNonInteractive, promptYesNoOrDefault, note }); |
| ); | ||
| }); | ||
|
|
||
| it("prompts Option 3 users and writes the Dockerfile ARG source env", async () => { |
| env: { | ||
| ...process.env, | ||
| HOME: tmpDir, | ||
| PATH: `${fakeBin}:${process.env.PATH || ""}`, |
b99fee0 to
9104d05
Compare
cd0f350 to
7be6ebf
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
🧹 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.tsshould be validated with the recommended E2E workflow job set (includingcloud-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, andissue-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
📒 Files selected for processing (6)
src/lib/onboard.tssrc/lib/onboard/compatible-endpoint-smoke.test.tssrc/lib/onboard/compatible-endpoint-smoke.tssrc/lib/onboard/reasoning-mode.test.tssrc/lib/onboard/reasoning-mode.tstest/onboard-compatible-reasoning-mode.test.ts
|
@coderabbitai summary |
✅ Action performedSummary regeneration triggered. |
Summary
NEMOCLAW_REASONING.reasoning_contentbut emptychoices[0].message.content.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.tsNODE_PATH=/Users/holim/code/NemoClaw/node_modules /Users/holim/code/NemoClaw/node_modules/.bin/tsc -p tsconfig.src.json --noEmitgit diff --checkFixes #3279
Summary by CodeRabbit
New Features
NEMOCLAW_REASONINGas"true"/"false", with input normalization and clear prompts.Bug Fixes
NEMOCLAW_REASONING=true).Tests