Skip to content

fix(bailian): support standard reasoning_effort none - #2414

Merged
llc1123 merged 2 commits into
looplj:unstablefrom
remote-controlled-man:fix/bailian-enable-thinking
Oct 7, 2026
Merged

llc1123 merged 2 commits into
looplj:unstablefrom
remote-controlled-man:fix/bailian-enable-thinking

Conversation

@remote-controlled-man

@remote-controlled-man remote-controlled-man commented Sep 6, 2026 •

Copy link
Copy Markdown
Contributor

Standard OpenAI clients can disable Bailian thinking with reasoning_effort: "none". The transformed request sends enable_thinking: false and omits reasoning_effort, following the documented none mapping: https://help.aliyun.com/en/model-studio/qwen-api-via-openai-chat-completions.

The adapter uses only the unified request's ReasoningEffort. Unspecified effort adds no thinking option; every other effort value is forwarded unchanged for the provider/model to interpret. The input request is not mutated, and consecutive tool-call merging is preserved.

Raw native enable_thinking remains a passthrough-only option. This replaces the original raw-field restoration approach in response to the maintainer's standard OpenAI format discussion. It does not claim to solve #2115's native-field use case without passthrough.

Validation: race-enabled, shuffled tests for all llm/transformer/... packages pass; scoped gopls check reports no diagnostics. JSON tests cover none, unspecified, other efforts, conflicting raw native flags, input immutability, and actual OpenAI inbound → Bailian outbound conversion. A separate import driver confirms the serialized outbound JSON for none, unspecified, minimal, low, medium, high, and xhigh.

Summary by CodeRabbit

  • Bug Fixes
    • Bailian requests now respect the selected reasoning effort: none disables thinking, other specified values are sent as the reasoning effort, and an unspecified value leaves reasoning settings unset.
    • This behavior is consistent for directly submitted requests and requests converted from OpenAI format. Conflicting thinking settings in the request body do not override the selected reasoning effort.

Copilot AI lite review requested due to automatic review settings September 6, 2026 07:51
@coderabbitai

coderabbitai Bot commented Sep 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: a6c4e59c-2e43-41aa-9f2c-73f86ce52c55

📥 Commits

Reviewing files that changed from the base of the PR and between 223e696 and d8e5c67.

📒 Files selected for processing (2)
  • llm/transformer/bailian/outbound.go
  • llm/transformer/bailian/outbound_test.go

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

The Bailian outbound transformer now maps ReasoningEffort == "none" to enable_thinking: false. It clears the effort value on a shallow copy before delegation and leaves the original request unchanged. Tests cover other effort values and OpenAI inbound requests.

Changes

Bailian request transformation

Layer / File(s) Summary
Transform and validate reasoning-effort serialization
llm/transformer/bailian/outbound.go, llm/transformer/bailian/outbound_test.go
For ReasoningEffort == "none", the transformer clears the value on a shallow copy, delegates the request, then sets enable_thinking to false in the generated body. Tests cover other effort values, conflicting raw flags, request immutability, and OpenAI inbound requests.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to d8e5c

The change supports disabling Bailian thinking through standard reasoning_effort requests without changing other effort values. No actionable merge-blocking risk is identified from the supplied evidence.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to d8e5c

The reviewed request-format change introduces no material security risk. It preserves existing validation and authentication, does not expand access or privileges, and leaves the original request unchanged.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated new effect is limited to reasoning-control serialization for requests already routed through Bailian with unified reasoning effort "none". The reviewed change does not introduce a new destination, credential source, or privilege transition.

Trust Boundaries and Controls

  • observed — Client-controlled reasoning effort is treated as a normalized request option, not as authority to change routing or authentication. On the reviewed transformed chat path, conflicting raw native flags cannot override the generated disabled-thinking value. This conclusion does not cover separate raw-passthrough behavior.

Resilience and Maintainability Implications

  • inferred — The added transformation is request-local: it preserves the caller's reasoning value and exposes the generated request only after the override succeeds. Its error paths do not strand shared state or require rollback, and repeated calls do not consume or rewrite the original reasoning setting.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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 and concisely describes the main change: adding support for standard reasoning_effort: "none" handling in Bailian.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

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.

🟢 Approval recommended

The change is narrowly scoped to Bailian OpenAI Chat outbound behavior and is covered by a focused unit test validating the intended forwarding rules.

Pull request overview

This PR fixes Bailian outbound transformation so that a Bailian-specific top-level enable_thinking boolean provided by OpenAI-compatible clients (via extra_body) is preserved through the OpenAI Chat Completions transformation, ensuring explicit false isn’t dropped.

Changes:

  • After delegating to the OpenAI outbound transformer, re-inject enable_thinking into the outbound JSON body only when the inbound raw request contains an explicit JSON boolean.
  • Leave enable_thinking absent when it’s omitted, null, non-boolean, or when the inbound API format is not OpenAI Chat Completions.
  • Add unit tests covering true, false, and various non-forwarded cases.
File summaries
File Description
llm/transformer/bailian/outbound.go Preserves explicit boolean enable_thinking from the inbound OpenAI Chat raw request by setting it back into the transformed outbound JSON body.
llm/transformer/bailian/outbound_test.go Adds coverage to verify boolean forwarding and non-forwarding behavior across omitted/null/non-boolean/non-OpenAI cases.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@looplj

looplj commented Sep 6, 2026

Copy link
Copy Markdown
Owner

有几个点,你要的是什么

  1. 兼容已有调用百炼的代码就行,那其实开透传即可。
  2. 已有百炼的代码,但是想接入其他渠道,这个比较麻烦,因为百炼只有一个 enable thinking 字段,无法映射到其他的 reasonning effort 的级别去。ds/glm 之类的,有默认的 reasonning effort 可能还行。
  3. 使用标准的 openai 格式调用百炼,参考:DeepSeek 怎么做的

@remote-controlled-man

Copy link
Copy Markdown
Contributor Author

更准确地说,是第 3 种里的窄场景:客户端按 OpenAI Chat Completions 调用 AxonHub,并通过 extra_body 在顶层携带百炼扩展字段 enable_thinking,请求再经非透传的百炼渠道转换。这个 PR 不做跨供应商或 reasoning_effort 映射;只保留显式 JSON boolean 的 true/false,省略、null、非 boolean 或非 OpenAI Chat 输入都不会注入。

@remote-controlled-man

remote-controlled-man commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Hi! This PR has been open for two weeks with all checks green, and I already posted the scope clarification on Sep 17 as you asked. Does the current scope look good to you? If it's not a fit right now, feel free to close it — no hard feelings on my side.

@llc1123
llc1123 force-pushed the fix/bailian-enable-thinking branch from a943e92 to c0feb24 Compare September 30, 2026 06:16
@llc1123

llc1123 commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

Rebased onto latest unstable (223e696); change content unchanged (range-diff identical). Verified: go test ./transformer/bailian and ./transformer/... pass; driver through Bailian outbound confirms enable_thinking true/false forwarded, null/string/number/omitted not introduced. New head c0feb24 CI (Build/Test/Lint) green.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@llc1123

llc1123 commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@llc1123 llc1123 changed the title fix(bailian): preserve enable_thinking from OpenAI requests fix(bailian): support standard reasoning_effort none Oct 1, 2026
@llc1123

llc1123 commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Updated this PR to the narrow standard OpenAI reasoning path discussed above. At d8e5c67, unified ReasoningEffort == "none" maps to enable_thinking: false, with outgoing reasoning_effort removed on a copy of the request. Other effort values are forwarded unchanged, and unspecified effort injects no thinking flag.

The official Bailian documentation describes none as mapping to enable_thinking=False; other effort support and defaults vary by model: https://help.aliyun.com/en/model-studio/qwen-api-via-openai-chat-completions. This patch therefore adds no true mapping, model lists, or unsupported-level filtering. Raw native enable_thinking remains passthrough-only, including the native-field scenario in #2115.

Verified all transformer packages with go test -race -shuffle=on -count=1 ./transformer/..., scoped gopls check, and a separate import driver through OpenAI inbound → Bailian outbound. Actual JSON for none: {"messages":[{"role":"user","content":"hi"}],"model":"qwen-max","enable_thinking":false}. Unspecified emits neither option; minimal/low/medium/high/xhigh retain reasoning_effort. Conflicting raw flags are ignored; input immutability and tool-call merge tests pass. The followup is signed by 粒粒橙 i@llc.moe with SSH fingerprint SHA256:MCdIIoZUa7KUrSQN7S/m80ISdLxAE1D2f1c/Sr2Wb38; the contributor's original commit and attribution are preserved.

@coderabbitai full review

@llc1123

llc1123 commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Final validation for d8e5c67: Build Backend, Build Frontend, Test Frontend Unit, lint, test, and CodeRabbit all passed. The fresh CodeRabbit review covers c0feb24..d8e5c67 and reports no actionable comments; there are no review threads. Independent requirements, execution QA, code-quality, security, and context reviews also passed. PR remains open and unmerged.

The summary's linked-issue and scope warnings still evaluate the original #2115 native-field proposal. The revised title/body and implementation intentionally follow the maintainer's standard OpenAI format path: only unified reasoning_effort: "none" maps to disabled thinking. #2115's native extra_body.enable_thinking scenario remains passthrough-dependent and is not claimed fixed. Restoring raw native fields would contradict this revised scope. The docstring warning is an advisory threshold on test functions; the modified exported transformer method already has its API comment. No implementation change is warranted for these advisory warnings.

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@llc1123

llc1123 commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

@looplj @remote-controlled-man 想请两位确认一下当前实现方向是否符合前面讨论的预期,先不合并。

当前 head 为 d8e5c67a。这次已移除“从原始请求恢复百炼专有 enable_thinking 字段”的实现,改为在百炼 outbound 边界处理统一模型中的标准 reasoning 控制:

  • 显式 reasoning_effort: "none":转换为 enable_thinking: false,同时移除出站 reasoning_effort。
  • 其他 effort 值:保留原值,不统一压成 enable_thinking: true,具体支持范围由百炼目标模型判断。
  • 未指定 effort:不注入 enable_thinking,保持供应商默认行为。
  • 客户端直接传百炼专有 enable_thinking 的场景:继续使用现有透传,不在转换路径里单独恢复原始字段。

参考百炼官方文档:https://help.aliyun.com/en/model-studio/qwen-api-via-openai-chat-completions 。文档说明部分模型支持标准 reasoning_effort,取值及默认值随模型而异,并明确描述了 none 与关闭思考的关系。因此此次只补齐标准关闭思考的转换,不增加模型白名单,也不把其他推理级别降为布尔开关。

这个方向对应前面讨论中的“通过标准 OpenAI reasoning 参数调用百炼”,并不宣称在关闭透传时保留原 issue #2115 的专有扩展字段。PR 标题和描述也已按这个范围更新。

实际 OpenAI inbound → 百炼 outbound 的 JSON 验证、transformer 测试及当前 head 的 CI 均已通过。请两位确认:是否认可上述边界,尤其是 none 的转换和专有字段仍由透传负责?若预期不同,请指出希望采用的参数契约,我们再据此调整。

@remote-controlled-man

Copy link
Copy Markdown
Contributor Author

The revised boundary looks right to me — confirming it.

  • reasoning_effort: "none" → enable_thinking: false (with reasoning_effort stripped on the wire) matches the documented none mapping and is the clean contract.
  • Forwarding every other effort value unchanged, and injecting nothing when effort is unspecified, keeps model-specific support where it belongs.
  • Leaving native extra_body.enable_thinking to passthrough channels is a reasonable scope call: restoring proprietary fields through the unified transform would be fragile, and it matches the direction from the earlier discussion.

One housekeeping question on #2115: since the revised PR intentionally does not fix the native-field scenario, how would you like the issue handled? I see two options: (a) keep it open, and I post a short note there documenting the two supported paths (a passthrough channel, or the standard reasoning_effort parameter), or (b) close it once this merges, with that explanation recorded in the issue. I'm happy to write the note either way.

Thanks for the careful rework — validating the none mapping against the actual outbound JSON is exactly the evidence this needed, and thanks for preserving the original commit's attribution.

@llc1123
llc1123 merged commit d6e00c0 into looplj:unstable Oct 7, 2026
6 checks passed
Edint386 pushed a commit to Edint386/axonhub that referenced this pull request Oct 9, 2026
* fix(bailian): preserve enable_thinking from OpenAI requests

* fix(bailian): map standard reasoning none to disabled thinking

---------

Co-authored-by: 粒粒橙 <i@llc.moe>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@remote-controlled-man
remote-controlled-man deleted the fix/bailian-enable-thinking branch October 13, 2026 01:28
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.

4 participants