Skip to content

feat(proxy): thinking effort 冲突整流器——兼容 DeepSeek/MiMo 子 agent 思考关闭报错 (#1257)#1269

Merged
ding113 merged 3 commits into
devfrom
feat/thinking-effort-conflict-rectifier
Jun 11, 2026
Merged

feat(proxy): thinking effort 冲突整流器——兼容 DeepSeek/MiMo 子 agent 思考关闭报错 (#1257)#1269
ding113 merged 3 commits into
devfrom
feat/thinking-effort-conflict-rectifier

Conversation

@ding113

@ding113 ding113 commented Jun 11, 2026

Copy link
Copy Markdown
Owner

Closes #1257

背景

Claude Code v2.1.166+ 为子 agent 任务主动关闭思考(thinking: {type: "disabled"}),但未剥离全局推理参数(output_config: {effort})。Anthropic 官方 API 忽略这种矛盾组合;DeepSeek 等严格校验的 Anthropic 兼容上游直接 400:

thinking options type cannot be disabled when reasoning_effort is set

issue 中引用的社区 proxy.js 即为此而生。本 PR 将其能力集成进 CCH 的整流器体系。

文档核实(国产模型可用性)

  • DeepSeek(api-docs.deepseek.com/guides/anthropic_api):thinking Supported(忽略 budget_tokens);output_config "Only effort is supported"——effort 内部映射为 reasoning_effort,即冲突组合确为「thinking.disabled + output_config.effort」,剥离 output_config 与文档语义一致。模型:deepseek-v4-pro / deepseek-v4-flash。
  • 小米 MiMo(platform.xiaomimimo.com):提供 Anthropic 兼容端点(/anthropic,mimo-v2.5-pro),thinking 经 reasoning_effort 启用;文档未记录该冲突的行为。由于整流器仅在上游真实返回该错误时触发,MiMo 若不报错则零影响,若报同类错误则同样被修复——被动触发天然保证了兼容安全。
  • CCH 自身的 anthropic_effort 特殊设置与 provider 参数覆写均围绕 output_config.effort 字段,与本整流器审计字段闭环。

实现(完全对齐 thinking-signature-rectifier 的被动触发模式)

核心(src/app/v1/_lib/proxy/thinking-effort-conflict-rectifier.ts):

  • detect*Trigger(errorMessage):仅字符串匹配("cannot be disabled" + reasoning_effort/output_config 变体),不依赖错误规则开关
  • rectify*(message):仅当 thinking 关闭(或缺省)时,剥离 output_config(携带 effort 时)与顶层 reasoning_effort 透传;保留 thinking 关闭状态(尊重客户端对子 agent 关闭思考的意图);thinking enabled/adaptive 或无 effort 字段时 applied=false 不重试

forwarder 集成:

  • tryApplyReactiveAnthropicRectifier 中置于签名整流器之前(更具体的错误检测优先,避免被其通用 invalid request 兜底吞掉);命中→整流→同供应商重试一次(retryState 防重复);hedge 分支自动复用
  • 特殊设置审计:新增 thinking_effort_conflict_rectifier 类型(hit/trigger/removedOutputConfig/removedReasoningEffort/thinkingType/effort),日志可回溯

系统设置(enableThinkingEffortConflictRectifier,默认开启):

  • Drizzle 列 + 迁移 drizzle/0105_chief_rocket_racer.sql(bun run db:generate 生成并已审查)
  • types / repository / transformers / settings-cache / zod validation / v1 API schema / server action / 设置页开关 UI / openapi 类型重新生成
  • i18n:5 语言 settings/config.json 各 2 个 key(遵循各语言既有表述风格)

测试(TDD,先红后绿;19 个新用例)

  • src/app/v1/_lib/proxy/thinking-effort-conflict-rectifier.test.ts(12):错误检测各变体与反例(签名/预算错误不误触)、整流各形态(含 thinking 缺省视为关闭、enabled/adaptive 不触碰、无 effort no-op)
  • tests/unit/proxy/proxy-forwarder-thinking-effort-conflict-rectifier.test.ts(4):命中→剥离→重试成功(且不误触签名整流器)、开关关闭不整流、not_applicable 不重试、同供应商最多重试一次
  • tests/unit/actions/system-config-thinking-effort-conflict-setting.test.ts(3):transformer 默认值、validation schema、v1 响应 schema
  • 新覆盖率配置 tests/configs/thinking-effort-conflict-rectifier.config.ts + package.json 脚本:96.9% Stmts / 89.5% Branch / 100% Funcs / 96.7% Lines(阈值 80/70/80/80)

验证

  • bun run build / typecheck / lint / test(根配置全量)/ test:v1 全部通过
  • 覆盖率配置:codex-session-id-completer / thinking-signature-rectifier / thinking-effort-conflict-rectifier(新)/ quota 通过;logs-sessionid-time-filter 在干净 dev 基线(499d925)上以相同数字(branches 85.52%,65/76)失败,为存量问题与本 PR 无关
  • openapi:check / openapi:lint 通过;i18n 键名守卫(sync-settings-keys / settings-split-guards)通过

🤖 Generated with Claude Code

Greptile Summary

This PR adds a new reactive "Thinking Effort Conflict Rectifier" that handles a specific 400 error from strict Anthropic-compatible providers (DeepSeek, MiMo) which reject the combination of thinking: {type: "disabled"} and reasoning_effort/output_config.effort. The fix detects the error by string-matching the upstream message, surgically strips only the conflicting effort fields, and retries once against the same provider.

  • The new rectifier (thinking-effort-conflict-rectifier.ts) is placed before the existing signature rectifier in forwarder.ts to prevent the more specific error from being swallowed by the generic invalid_request fallback; the database column, schema, transformers, fallback-chain, settings cache, settings UI, and i18n strings are all updated consistently.
  • The previous reviewer's concern about deleting the entire output_config object has been addressed — the new implementation surgically removes only the effort key and drops the object only when it becomes empty.

Confidence Score: 5/5

Safe to merge — the rectifier is passively triggered only on an exact 400 error string, touches no normal-path requests, and the one retry is guarded against repetition.

All changed paths are well-isolated: the new rectifier fires only after an upstream 400 containing the specific DeepSeek/MiMo error wording, leaves thinking-disabled intact, and retries at most once. The database fallback chain is correctly extended with the new column at the outermost tier. Tests cover detection variants, rectification edge-cases (sibling-key preservation, enabled/adaptive no-ops, effort-less no-ops), the disabled-switch path, and the retry-once guard. No existing behavior is altered for providers that don't return this specific error.

No files require special attention.

Important Files Changed

Filename Overview
src/app/v1/_lib/proxy/thinking-effort-conflict-rectifier.ts New core rectifier; detection and rectification logic is well-scoped; surgical effort-field removal correctly preserves sibling keys in output_config.
src/app/v1/_lib/proxy/forwarder.ts New rectifier wired before signature rectifier (correct priority); retry-state flag and hedge-branch initialization both updated; fallthrough behaviour when disabled is consistent with sibling rectifiers.
src/repository/system-config.ts New column added to both read and write fallback chains; if (!updated) guard correctly prevents the billHedgeLosers secondary fallback from executing if the new effortConflict fallback already succeeded.
src/types/special-settings.ts New audit type added and union updated; fields mirror the rectifier result cleanly.
drizzle/0105_chief_rocket_racer.sql Single ALTER TABLE adding the boolean column with DEFAULT true NOT NULL — matches schema.ts and is safe for live migration.
tests/unit/proxy/proxy-forwarder-thinking-effort-conflict-rectifier.test.ts 4 integration tests cover hit+retry, disabled switch, not_applicable, and retry-once guard; session mock is thorough.
src/app/v1/_lib/proxy/thinking-effort-conflict-rectifier.test.ts 12 unit tests covering detection variants, rectification edge-cases including sibling-key preservation, enabled/adaptive no-op, and effort-less no-op.
tests/unit/repository/system-config-update-missing-columns.test.ts New regression test verifies the new column appears at the outermost fallback tier and that the second select call strips it (not billHedgeLosers).

Sequence Diagram

sequenceDiagram
    participant CC as Claude Code (subagent)
    participant PF as ProxyForwarder
    participant TECR as ThinkingEffortConflictRectifier
    participant DS as DeepSeek / MiMo

    CC->>PF: "POST /v1/messages {thinking:{type:"disabled"}, output_config:{effort:"max"}}"
    PF->>DS: Forward request (attempt 1)
    DS-->>PF: 400 thinking options type cannot be disabled when reasoning_effort is set

    PF->>TECR: detectThinkingEffortConflictRectifierTrigger(errorMsg)
    TECR-->>PF: thinking_disabled_with_reasoning_effort

    PF->>TECR: rectifyThinkingEffortConflict(message)
    Note over TECR: Remove output_config.effort, keep sibling keys, remove top-level reasoning_effort, keep thinking disabled
    TECR-->>PF: "applied=true, removedOutputConfig=true"

    PF->>PF: "retryState.thinkingEffortConflictRetried = true"
    PF->>PF: persist special setting audit log

    PF->>DS: "Forward request (attempt 2) {thinking:{type:"disabled"}, messages:[...]}"
    DS-->>PF: 200 OK
    PF-->>CC: 200 OK
Loading

Reviews (3): Last reviewed commit: "test(repository): assert stripped select..." | Re-trigger Greptile

When Anthropic-compatible providers like DeepSeek or MiMo receive a request
with thinking disabled but reasoning_effort or output_config.effort set,
they return a 400 error. This rectifier detects the specific error message,
strips the conflicting effort fields while preserving the disabled thinking
state, and retries the request once against the same provider.

Adds a new system setting enableThinkingEffortConflictRectifier (enabled by
default) with full UI, API, and database migration support to control this
behavior.
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

该PR实现了完整的 thinking effort 冲突自动整流与重试:当 Anthropic 兼容供应商因 thinking disabled 与 reasoning_effort 并存返回 400 时,系统可按配置移除冲突字段并对同一提供方重试,变更涵盖数据库、API/验证、缓存与仓库、核心 rectifier、Proxy 集成、UI 与测试。

Changes

Thinking Effort 冲突整流器完整实现

Layer / File(s) Summary
数据库架构与迁移
drizzle/0105_chief_rocket_racer.sql, drizzle/meta/_journal.json, src/drizzle/schema.ts
新增数据库迁移列 enable_thinking_effort_conflict_rectifier(boolean, NOT NULL, 默认 true),并记录 journal 条目。
API 类型与验证
src/lib/api/v1/schemas/system-config.ts, src/lib/api-client/v1/openapi-types.gen.ts, src/lib/validation/schemas.ts, src/types/system-config.ts
在 SystemSettings / 更新输入 / OpenAPI types / Zod 校验中新增 enableThinkingEffortConflictRectifier 布尔字段与描述。
系统设置缓存与 transformer
src/lib/config/system-settings-cache.ts, src/repository/_shared/transformers.ts
DEFAULT_SETTINGS 与 getCachedSystemSettings 的默认值、toSystemSettings 的 DB->API 映射中新增该字段与默认回退。
仓库读写与降级逻辑
src/repository/system-config.ts, tests/unit/repository/system-config-update-missing-columns.test.ts
get/updateSystemSettings 的 projection/returning 与更新构建中加入该字段;在列缺失降级链中将其作为最新一层被剔除并相应添加回归测试。
特殊设置类型与 key 构建
src/types/special-settings.ts, src/lib/utils/special-settings.ts
新增 ThinkingEffortConflictRectifierSpecialSetting 类型与 buildSettingKey 的 case 支持,包含 hit/providerId/trigger/attempts/removed 信息等字段。
核心整流器实现与单元测试
src/app/v1/_lib/proxy/thinking-effort-conflict-rectifier.ts, src/app/v1/_lib/proxy/thinking-effort-conflict-rectifier.test.ts
新增触发器检测与 rectify 函数:匹配错误文本并在 thinking 为 disabled 时移除 output_config.effortreasoning_effort,返回结构化结果;含详尽测试覆盖匹配与修复行为。
ProxyForwarder 集成与测试
src/app/v1/_lib/proxy/forwarder.ts, tests/unit/proxy/proxy-forwarder-thinking-effort-conflict-rectifier.test.ts
在 tryApplyReactiveAnthropicRectifier 中接入新 rectifier、加入 retry 幂等字段 thinkingEffortConflictRetried、按开关控制并写入 special setting;新增集成单测覆盖重试、写入与幂等行为。
UI、动作与国际化文案
src/actions/system-config.ts, src/app/[locale]/settings/config/_components/system-settings-form.tsx, src/app/[locale]/settings/config/page.tsx, messages/{en,ja,ru,zh-CN,zh-TW}/settings/config.json
后端 saveSystemSettings 接收新字段;设置表单新增开关与状态、提交/保存流程更新;页面传参补齐;五种语言添加标题与描述文案。
测试覆盖配置与脚本
tests/configs/thinking-effort-conflict-rectifier.config.ts, package.json, tests/unit/actions/system-config-thinking-effort-conflict-setting.test.ts
新增覆盖率配置与 npm script、以及单元测试验证 transformer 默认、验证 schema 与 API schema 暴露。

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes


Possibly related PRs

  • ding113/claude-code-hub#945: Both PRs modify src/app/v1/_lib/proxy/forwarder.ts to integrate Anthropic “reactive rectifier + retry” logic into hedge/streaming flow—main PR adds a new thinking_effort_conflict_rectifier type and retry-state wiring on top of the reactive-rectifier framework introduced in #945.
  • ding113/claude-code-hub#1098: Two PRs are related because both modify src/repository/system-config.ts in the getSystemSettings retrieval logic—main PR adds the new enableThinkingEffortConflictRectifier column handling while the retrieved PR changes the LIMIT 1 query to deterministically orderBy(asc(systemSettings.id)), so they touch the same query-building code path.
  • ding113/claude-code-hub#979: 两者都影响 src/app/v1/_lib/proxy/forwarder.ts 中的“特殊设置/调试工件”持久化路径,代码路径相关。
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed 标题清晰准确地反映了 PR 的主要变更:添加了 thinking effort 冲突整流器以兼容 DeepSeek/MiMo 报错。
Linked Issues check ✅ Passed PR 完全满足 issue #1257 的需求:检测特定错误、剥离冲突字段、允许重试,实现了被动整流能力。
Out of Scope Changes check ✅ Passed 所有变更均在 thinking effort 冲突整流的范围内,包括核心检测/整流逻辑、系统设置、数据库迁移、UI、i18n 和测试。
Description check ✅ Passed PR 描述清晰地关联了变更集。详细说明了背景(DeepSeek/MiMo 在思考关闭时返回 400 错误)、实现细节、测试覆盖和验证过程。

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

✨ 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 feat/thinking-effort-conflict-rectifier

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 and usage tips.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces the 'Thinking Effort Conflict Rectifier', a feature designed to resolve 400 Bad Request errors from strict Anthropic-compatible providers (such as DeepSeek or MiMo) when thinking is disabled but reasoning effort configurations are still present in the payload. The changes include database schema updates, UI settings, localization, and proxy forwarder integration to strip conflicting effort fields and retry requests. Feedback on the changes highlights a critical database fallback compatibility issue in src/repository/system-config.ts that could cause errors during rolling updates before migrations run, as well as a recommendation to refine the rectifier logic to selectively delete the effort field rather than the entire output_config object to ensure future compatibility.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

interceptAnthropicWarmupRequests: systemSettings.interceptAnthropicWarmupRequests,
enableThinkingSignatureRectifier: systemSettings.enableThinkingSignatureRectifier,
enableThinkingBudgetRectifier: systemSettings.enableThinkingBudgetRectifier,
enableThinkingEffortConflictRectifier: systemSettings.enableThinkingEffortConflictRectifier,

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.

high

getSystemSettings 中,新引入的 enableThinkingEffortConflictRectifier 字段被直接加入到了 selectionWithoutHighConcurrencyMode 中。

然而,在 selectSettingsRow 的数据库降级兼容链(fallback chain)中,最外层的降级(第 309 行左右)仍然是针对上一次新增的 billHedgeLosers 进行的:

const { billHedgeLosers: _omitBillHedgeLosers, ...selectionWithoutBillHedgeLosers } = fullSelection;

由于 fullSelection 包含了 selectionWithoutHighConcurrencyMode,如果数据库尚未执行本次迁移(例如在滚动更新或尚未运行 db:generate 的环境中),查询 enableThinkingEffortConflictRectifier 会直接报错。此时降级链会尝试移除 billHedgeLosers,但由于新字段 enableThinkingEffortConflictRectifier 依然存在于 selectionWithoutBillHedgeLosers 中,后续的所有降级尝试都将持续报错,最终一路溃退到最底层的 minimalSelection

这会导致在数据库迁移完成前,所有中间版本引入的系统设置(如高并发模式、IP 提取配置等)全部失效。

建议:
selectSettingsRow 的降级链最顶端,优先剥离本次新增的 enableThinkingEffortConflictRectifier 字段,然后再依次向下剥离旧字段。例如:

// 最新降级:移除最近新增的 enableThinkingEffortConflictRectifier 列
const { enableThinkingEffortConflictRectifier: _omitThinkingEffort, ...selectionWithoutThinkingEffort } = fullSelection;

// 随后在 selectionWithoutThinkingEffort 的基础上继续剥离旧列...

(注:由于该函数未在本次 Diff 的修改范围内,无法直接提供行内代码建议,请手动在 selectSettingsRow 中进行调整。)

Comment on lines +64 to +113
export function rectifyThinkingEffortConflict(
message: Record<string, unknown>
): ThinkingEffortConflictRectifierResult {
const thinking = message.thinking;
const thinkingType =
thinking && typeof thinking === "object" && !Array.isArray(thinking)
? typeof (thinking as Record<string, unknown>).type === "string"
? ((thinking as Record<string, unknown>).type as string)
: null
: null;

const result: ThinkingEffortConflictRectifierResult = {
applied: false,
removedOutputConfig: false,
removedReasoningEffort: false,
thinkingType,
effort: null,
};

// thinking 显式启用(enabled/adaptive 等)时不属于该冲突,保持原样
const thinkingDisabled = thinkingType === null || thinkingType === "disabled";
if (!thinkingDisabled) {
return result;
}

const outputConfig = message.output_config;
const outputConfigEffort =
outputConfig && typeof outputConfig === "object" && !Array.isArray(outputConfig)
? (outputConfig as Record<string, unknown>).effort
: undefined;

if (outputConfigEffort !== undefined) {
result.effort = typeof outputConfigEffort === "string" ? outputConfigEffort : null;
delete message.output_config;
result.removedOutputConfig = true;
result.applied = true;
}

const reasoningEffort = message.reasoning_effort;
if (reasoningEffort !== undefined) {
if (result.effort === null && typeof reasoningEffort === "string") {
result.effort = reasoningEffort;
}
delete message.reasoning_effort;
result.removedReasoningEffort = true;
result.applied = true;
}

return result;
}

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.

medium

在整流 output_config 时,直接使用 delete message.output_config 会将整个 output_config 对象删除。如果未来 Anthropic 或兼容上游在 output_config 中引入了除 effort 之外的其他配置字段,这种暴力的删除方式会导致其他有用的配置字段丢失。

此外,为了增强代码的健壮性,建议在函数入口处添加对 message 的防御性空值与类型校验,防止在运行时因传入非对象而抛出 TypeError

建议对该函数进行重构:

  1. 在入口处添加对 message 的防御性校验。
  2. 仅删除 output_config 中的 effort 属性。如果删除后 output_config 变为空对象,再将其从 message 中彻底删除。这样可以保证更好的前向兼容性和鲁棒性。
export function rectifyThinkingEffortConflict(
  message: Record<string, unknown>
): ThinkingEffortConflictRectifierResult {
  const result: ThinkingEffortConflictRectifierResult = {
    applied: false,
    removedOutputConfig: false,
    removedReasoningEffort: false,
    thinkingType: null,
    effort: null,
  };

  if (!message || typeof message !== "object") {
    return result;
  }

  const thinking = message.thinking;
  const thinkingType =
    thinking && typeof thinking === "object" && !Array.isArray(thinking)
      ? typeof (thinking as Record<string, unknown>).type === "string"
        ? ((thinking as Record<string, unknown>).type as string)
        : null
      : null;

  result.thinkingType = thinkingType;

  // thinking 显式启用(enabled/adaptive 等)时不属于该冲突,保持原样
  const thinkingDisabled = thinkingType === null || thinkingType === "disabled";
  if (!thinkingDisabled) {
    return result;
  }

  const outputConfig = message.output_config;
  const outputConfigEffort =
    outputConfig && typeof outputConfig === "object" && !Array.isArray(outputConfig)
      ? (outputConfig as Record<string, unknown>).effort
      : undefined;

  if (outputConfigEffort !== undefined) {
    result.effort = typeof outputConfigEffort === "string" ? outputConfigEffort : null;
    const outputConfigObj = { ...(outputConfig as Record<string, unknown>) };
    delete outputConfigObj.effort;
    if (Object.keys(outputConfigObj).length === 0) {
      delete message.output_config;
    } else {
      message.output_config = outputConfigObj;
    }
    result.removedOutputConfig = true;
    result.applied = true;
  }

  const reasoningEffort = message.reasoning_effort;
  if (reasoningEffort !== undefined) {
    if (result.effort === null && typeof reasoningEffort === "string") {
      result.effort = reasoningEffort;
    }
    delete message.reasoning_effort;
    result.removedReasoningEffort = true;
    result.applied = true;
  }

  return result;
}

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f70b7832c9

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".


if (outputConfigEffort !== undefined) {
result.effort = typeof outputConfigEffort === "string" ? outputConfigEffort : null;
delete message.output_config;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve other output_config fields when stripping effort

When a retried request has output_config containing effort plus any other option, this deletes the entire object even though the rectifier only needs to remove the effort carrier. The rest of the codebase explicitly preserves unknown output_config properties when applying Anthropic overrides, so this retry can silently drop provider/client parameters on the second attempt instead of only resolving the thinking/effort conflict.

Useful? React with 👍 / 👎.

interceptAnthropicWarmupRequests: systemSettings.interceptAnthropicWarmupRequests,
enableThinkingSignatureRectifier: systemSettings.enableThinkingSignatureRectifier,
enableThinkingBudgetRectifier: systemSettings.enableThinkingBudgetRectifier,
enableThinkingEffortConflictRectifier: systemSettings.enableThinkingEffortConflictRectifier,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Add the new column to the unmigrated DB fallback path

If the app starts with code from this commit before migration 0105 has run, the first select fails because this new column is missing; the undefined-column fallback below removes older columns such as billHedgeLosers first but keeps enableThinkingEffortConflictRectifier in every wide fallback, so it keeps failing until the minimal selection and loses persisted settings by replacing them with defaults. Please omit this new column in the first fallback tier so older system_settings rows still load all existing columns during rolling deploys or manual migrations.

Useful? React with 👍 / 👎.

Comment on lines +95 to +100
if (outputConfigEffort !== undefined) {
result.effort = typeof outputConfigEffort === "string" ? outputConfigEffort : null;
delete message.output_config;
result.removedOutputConfig = true;
result.applied = true;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 delete message.output_config removes the entire object when any effort key is found. If a provider ever puts fields other than effort inside output_config (e.g. a future extension), those extra fields are silently discarded. Surgically removing only the effort key and then conditionally cleaning up an empty shell is safer.

Suggested change
if (outputConfigEffort !== undefined) {
result.effort = typeof outputConfigEffort === "string" ? outputConfigEffort : null;
delete message.output_config;
result.removedOutputConfig = true;
result.applied = true;
}
if (outputConfigEffort !== undefined) {
result.effort = typeof outputConfigEffort === "string" ? outputConfigEffort : null;
delete (outputConfig as Record<string, unknown>).effort;
if (Object.keys(outputConfig as Record<string, unknown>).length === 0) {
delete message.output_config;
}
result.removedOutputConfig = true;
result.applied = true;
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/thinking-effort-conflict-rectifier.ts
Line: 95-100

Comment:
`delete message.output_config` removes the entire object when any `effort` key is found. If a provider ever puts fields other than `effort` inside `output_config` (e.g. a future extension), those extra fields are silently discarded. Surgically removing only the `effort` key and then conditionally cleaning up an empty shell is safer.

```suggestion
  if (outputConfigEffort !== undefined) {
    result.effort = typeof outputConfigEffort === "string" ? outputConfigEffort : null;
    delete (outputConfig as Record<string, unknown>).effort;
    if (Object.keys(outputConfig as Record<string, unknown>).length === 0) {
      delete message.output_config;
    }
    result.removedOutputConfig = true;
    result.applied = true;
  }
```

How can I resolve this? If you propose a fix, please make it concise.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/types/system-config.ts (1)

198-206: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

UpdateSystemSettingsInput 缺少新字段,导致类型检查失败(构建阻断)

SystemSettings 已在 Line 94-97 增加 enableThinkingEffortConflictRectifier,但 UpdateSystemSettingsInput 未同步增加该可选字段,直接触发 src/actions/system-config.ts Line 129 的 TS2353。请在输入类型中补齐该字段,保持跨层契约一致。

🔧 建议修复
 export interface UpdateSystemSettingsInput {
@@
   // thinking budget 整流器(可选)
   enableThinkingBudgetRectifier?: boolean;
+
+  // thinking effort 冲突整流器(可选)
+  enableThinkingEffortConflictRectifier?: boolean;
 
   // billing header 整流器(可选)
   enableBillingHeaderRectifier?: boolean;
🤖 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/types/system-config.ts` around lines 198 - 206, SystemSettings 增加了
enableThinkingEffortConflictRectifier,但 UpdateSystemSettingsInput
未同步,导致类型不匹配;在类型定义 UpdateSystemSettingsInput 中新增可选字段
enableThinkingEffortConflictRectifier?: boolean,使其与 SystemSettings
对齐,修复使用该输入类型(例如在 system-config actions 中处理更新设置的函数)时触发的 TS2353 类型错误。

Sources: Linters/SAST tools, Pipeline failures

🧹 Nitpick comments (1)
src/app/v1/_lib/proxy/thinking-effort-conflict-rectifier.test.ts (1)

60-77: ⚡ Quick win

建议补一个“output_config 含多字段”回归用例。

当前用例没有覆盖 output_config: { effort: "...", other: ... } 场景。建议断言整流后仅移除 effort,其余字段保留,防止后续回归成整块删除。

可补充的测试示例
+  test("only removes output_config.effort and preserves other output_config fields", () => {
+    const message: Record<string, unknown> = {
+      thinking: { type: "disabled" },
+      output_config: { effort: "max", keep_me: true },
+      messages: [],
+    };
+
+    const result = rectifyThinkingEffortConflict(message);
+
+    expect(result.applied).toBe(true);
+    expect(result.removedOutputConfig).toBe(true);
+    expect(message.output_config).toEqual({ keep_me: true });
+  });
🤖 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/app/v1/_lib/proxy/thinking-effort-conflict-rectifier.test.ts` around
lines 60 - 77, Add a regression test for rectifyThinkingEffortConflict that uses
an input message where output_config has multiple fields (e.g., { effort: "max",
other: "keep" }); call rectifyThinkingEffortConflict and assert it applied, that
only the reasoning effort was removed (removedReasoningEffort true) while the
output_config key itself remains (removedOutputConfig false), the remaining
fields (other) are preserved, the effort value is reported in the result, and
message.thinking remains { type: "disabled" }; reference
rectifyThinkingEffortConflict and the existing test name "removes output_config
when thinking is disabled (Claude Code subagent shape)" to add a similar test
covering the multi-field output_config case.
🤖 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.

Inline comments:
In `@src/app/v1/_lib/proxy/thinking-effort-conflict-rectifier.ts`:
- Around line 95-99: The code currently deletes message.output_config entirely
when outputConfigEffort is detected; instead only remove the conflicting effort
field: if outputConfigEffort !== undefined set result.effort appropriately,
delete message.output_config.effort (not message.output_config), set
result.removedOutputConfig (or preferably rename/update it to indicate the
effort field was removed) and result.applied = true; update any downstream logic
that relied on whole-object deletion to tolerate the partial removal
(references: outputConfigEffort, message.output_config, result.effort,
result.removedOutputConfig, result.applied).

In `@src/repository/system-config.ts`:
- Around line 759-762: The code reads
payload.enableThinkingEffortConflictRectifier but the UpdateSystemSettingsInput
type is missing that optional field, causing type-check failures; add
enableThinkingEffortConflictRectifier?: boolean to the UpdateSystemSettingsInput
definition and any upstream API/input types (and corresponding schema/action
types) so the new boolean flag is accepted through the type chain, then update
any related input/interface (e.g., the payload consumer and system settings
update handlers) to match the new property name.

---

Outside diff comments:
In `@src/types/system-config.ts`:
- Around line 198-206: SystemSettings 增加了
enableThinkingEffortConflictRectifier,但 UpdateSystemSettingsInput
未同步,导致类型不匹配;在类型定义 UpdateSystemSettingsInput 中新增可选字段
enableThinkingEffortConflictRectifier?: boolean,使其与 SystemSettings
对齐,修复使用该输入类型(例如在 system-config actions 中处理更新设置的函数)时触发的 TS2353 类型错误。

---

Nitpick comments:
In `@src/app/v1/_lib/proxy/thinking-effort-conflict-rectifier.test.ts`:
- Around line 60-77: Add a regression test for rectifyThinkingEffortConflict
that uses an input message where output_config has multiple fields (e.g., {
effort: "max", other: "keep" }); call rectifyThinkingEffortConflict and assert
it applied, that only the reasoning effort was removed (removedReasoningEffort
true) while the output_config key itself remains (removedOutputConfig false),
the remaining fields (other) are preserved, the effort value is reported in the
result, and message.thinking remains { type: "disabled" }; reference
rectifyThinkingEffortConflict and the existing test name "removes output_config
when thinking is disabled (Claude Code subagent shape)" to add a similar test
covering the multi-field output_config case.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 844fca13-9133-4071-b0dc-3827913e84ff

📥 Commits

Reviewing files that changed from the base of the PR and between 499d925 and f70b783.

📒 Files selected for processing (28)
  • drizzle/0105_chief_rocket_racer.sql
  • drizzle/meta/0105_snapshot.json
  • drizzle/meta/_journal.json
  • messages/en/settings/config.json
  • messages/ja/settings/config.json
  • messages/ru/settings/config.json
  • messages/zh-CN/settings/config.json
  • messages/zh-TW/settings/config.json
  • package.json
  • src/actions/system-config.ts
  • src/app/[locale]/settings/config/_components/system-settings-form.tsx
  • src/app/[locale]/settings/config/page.tsx
  • src/app/v1/_lib/proxy/forwarder.ts
  • src/app/v1/_lib/proxy/thinking-effort-conflict-rectifier.test.ts
  • src/app/v1/_lib/proxy/thinking-effort-conflict-rectifier.ts
  • src/drizzle/schema.ts
  • src/lib/api-client/v1/openapi-types.gen.ts
  • src/lib/api/v1/schemas/system-config.ts
  • src/lib/config/system-settings-cache.ts
  • src/lib/utils/special-settings.ts
  • src/lib/validation/schemas.ts
  • src/repository/_shared/transformers.ts
  • src/repository/system-config.ts
  • src/types/special-settings.ts
  • src/types/system-config.ts
  • tests/configs/thinking-effort-conflict-rectifier.config.ts
  • tests/unit/actions/system-config-thinking-effort-conflict-setting.test.ts
  • tests/unit/proxy/proxy-forwarder-thinking-effort-conflict-rectifier.test.ts

Comment thread src/app/v1/_lib/proxy/thinking-effort-conflict-rectifier.ts
Comment thread src/repository/system-config.ts
@github-actions github-actions Bot added the size/XL Extra Large PR (> 1000 lines) label Jun 11, 2026

@github-actions github-actions 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.

Code Review Summary

This PR adds a reactive "thinking effort conflict rectifier" that handles the incompatibility between thinking: { type: "disabled" } and output_config: { effort } / reasoning_effort — a combination that strict Anthropic-compatible providers (DeepSeek, MiMo) reject with 400. The implementation is clean, follows existing rectifier patterns precisely, and has comprehensive test coverage (96.9% statements). No issues meeting the reporting threshold were found.

PR Size: XL

  • Lines changed: 5,327 (note: ~4,500 are auto-generated Drizzle snapshot; actual meaningful changes ~500 lines)
  • Files changed: 28 (note: 3 are Drizzle auto-generated metadata)

Issues Found

Category Critical High Medium Low
Logic/Bugs 0 0 0 0
Security 0 0 0 0
Error Handling 0 0 0 0
Types 0 0 0 0
Comments/Docs 0 0 0 0
Tests 0 0 0 0
Simplification 0 0 0 0

Observations (below reporting threshold)

  1. delete message.output_config removes the entire object (confidence: ~70): When output_config contains effort alongside other keys, the whole object is deleted rather than just the effort key. The PR author acknowledges this in the description and Greptile's review also flagged it. In practice, DeepSeek only puts effort in output_config, so this is a theoretical concern for future providers — not a current bug. The test suite explicitly verifies that effort-less output_config objects are preserved, which covers the main alternative case.

Review Coverage

  • Logic and correctness - Clean
  • Security (OWASP Top 10) - Clean
  • Error handling - Clean
  • Type safety - Clean
  • Documentation accuracy - Clean
  • Test coverage - Excellent (96.9% stmts, 89.5% branches, 100% functions, 19 new tests)
  • Code clarity - Clean

Key Verification Points

  • Rectifier ordering in forwarder: effort conflict checked before signature rectifier (correct — more specific detection wins)
  • Retry-once guard via thinkingEffortConflictRetried flag (consistent with existing rectifiers)
  • All 5 i18n locales (en, ja, ru, zh-CN, zh-TW) have matching key pairs
  • Settings infrastructure: DB migration, schema, transformer, cache defaults, validation schema, API schema, server action, UI toggle — all consistent
  • Feature toggle defaults to enabled with proper fallback in all code paths (cache miss, DB read failure, transformer)

Automated review by Claude AI

Add missing enableThinkingEffortConflictRectifier field to
UpdateSystemSettingsInput to resolve a typecheck failure.

Strip only the effort key from output_config in the thinking effort
conflict rectifier instead of deleting the entire object, preserving
any sibling configuration fields.

Extend the system_settings database degradation fallback chain to
handle missing enableThinkingEffortConflictRectifier columns in
un-migrated databases for both read and update operations.
@github-actions

Copy link
Copy Markdown
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

The previous assertion only checked the transformer default value for the new column, which did not guarantee the fallback query actually stripped the correct column. The test now inspects the second select call to ensure the newly added column is removed while older fallback columns remain, catching regressions in the degradation chain order.
@github-actions

Copy link
Copy Markdown
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

@ding113

ding113 commented Jun 11, 2026

Copy link
Copy Markdown
Owner Author

评审驱动的修复(deep-review + /code-review max effort)

经多代理评审 + 机器人评论交叉核对,本 PR 已修复以下真实问题(均已 push,CI 全绿):

  1. 编译失败(Critical,CodeRabbit 命中)UpdateSystemSettingsInput 缺少 enableThinkingEffortConflictRectifier 字段,而 updateSystemSettings 读取了它 → tsgo 报 3 处 TS2339/TS2353。已补字段。(注:此前本地"typecheck 绿"是假绿——验证命令把 tsgo 通过 | tail 管道导致退出码被 tail 掩盖;vitest/esbuild 不做类型检查,故单测仍绿。已修正验证方式并以真实退出码复跑。)

  2. output_config 整体删除过宽(gemini/codex/greptile/CodeRabbit 均命中) — 原 delete message.output_config 会误删非 effort 的兄弟字段。改为仅剥离 output_config.effort,剥离后为空对象才整体移除;新增 2 个单测覆盖"保留兄弟字段"与"空对象移除"。

  3. 未迁移库的降级链漏列(gemini HIGH + codex/greptile/CodeRabbit) — 新列未加入 system_settings 读/写降级兜底链最外层,先于迁移部署时会导致所有设置读写失败。已在读链与更新链均加入该列的最外层降级(更新链同步补 if (!updated) 守卫),并新增回归测试。

  4. 回归测试形同虚设(/code-review sweep 命中) — 上一版降级回归测试只断言调用次数 + transformer 默认值,回退修复后仍会通过。已改为断言第二次 select 恰好剥离了新列、保留更旧列;并做了变异测试(回退修复 → 测试失败)确认其真正守护。

未在本 PR 内处理(说明)

  • 降级链应数据驱动(altitude/cleanup):手写的逐列降级阶梯是 claudecode新版本在agent里使用deepseek会因为思考模式报错,可以考虑兼容下 #1257 这类"加列忘改链"的根源,理想方案是改为按有序列数组循环。但读/更新两条链尾部有不规则的多列手建基集,完整重构约涉 ~300 行关键设置基础设施,属于纯清理(非缺陷),不宜混入功能 PR;当前列的复发风险已由上述回归测试守护。建议作为独立后续重构 PR。

@ding113 ding113 merged commit 53fa518 into dev Jun 11, 2026
10 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Claude Code Hub Roadmap Jun 11, 2026
00010110 added a commit to 00010110/claude-code-hub that referenced this pull request Jun 11, 2026
…nflict rectifier)

- 迁移序号冲突:将本 PR 的 system-message-rectifier 列迁移移至 0106
  (drizzle 重新生成为 0106_stiff_dormammu.sql),让出 0105 给 dev 已合入
  的 0105_chief_rocket_racer.sql(thinking-effort-conflict rectifier)。
- src/repository/system-config.ts 降级链堆叠:
  最新层先剥离 enableSystemMessageRectifier,次层再剥离
  enableThinkingEffortConflictRectifier,再回到原有 billHedgeLosers 等
  既有降级路径,确保两轮新增列对老库的兼容性。
- tests/unit/repository/system-config-update-missing-columns.test.ts:
  原 "仅缺 enable_thinking_effort_conflict_rectifier 列" 测试改为
  覆盖 enableSystemMessageRectifier(最外层),新增一项覆盖连续剥离两
  个新列直到 billHedgeLosers 层的回归断言。
@github-actions github-actions Bot mentioned this pull request Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core area:i18n area:provider enhancement New feature or request size/XL Extra Large PR (> 1000 lines)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant