Skip to content

feat(sdk): add fallback_model and proxy options to Python and TS SDKs - #6478

Closed
juhuan wants to merge 1 commit into
QwenLM:mainfrom
juhuan:feat/sdk-output-format-proxy-fallback
Closed

feat(sdk): add fallback_model and proxy options to Python and TS SDKs#6478
juhuan wants to merge 1 commit into
QwenLM:mainfrom
juhuan:feat/sdk-output-format-proxy-fallback

Conversation

@juhuan

@juhuan juhuan commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add fallback_model (Python) / fallbackModel (TS) option mapping to CLI's --fallback-model flag (max 3 models, comma-separated)
  • Add proxy option mapping to CLI's --proxy flag (deprecated, but still functional)
  • Both options implemented in Python SDK (types, validation, transport) and TS SDK (types, schema, transport, createQuery)
  • Unit tests added for both SDKs covering CLI argument building, validation, and pass-through

Test plan

  • Python SDK unit tests pass (37 tests)
  • TS SDK unit tests pass (79 tests)
  • fallbackModel rejects >3 entries via Zod schema
  • fallback_model rejects >3 entries via Python validation
  • Empty proxy string rejected by both SDKs

Expose CLI's --fallback-model (capacity-error fallback, max 3) and
--proxy (deprecated network proxy) flags through both SDKs' QueryOptions,
with validation and unit tests.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi @juhuan — thanks for the PR! Before we dive into the code, we need the PR body to follow the PR template. The current description uses ## Summary / ## Test plan headings, but the template requires:

  • ## What this PR does — describe the change in prose
  • ## Why it's needed — motivation and user-facing benefit
  • ## Reviewer Test Plan with ### How to verify, ### Evidence (Before & After), and ### Tested on subsections
  • ## Risk & Scope — main risks, out-of-scope items, breaking changes
  • ## Linked Issues — related issues
  • Chinese translation in a <details> block

These headings aren't bureaucracy — they help reviewers understand why the change exists and how to verify it without guessing. Could you update the PR body to match the template?

中文说明

@juhuan 你好,感谢你的 PR!在审查代码之前,需要 PR 正文遵循 PR 模板。当前描述使用了 ## Summary / ## Test plan 标题,但模板要求:

  • ## What this PR does — 用文字描述变更内容
  • ## Why it's needed — 动机和用户价值
  • ## Reviewer Test Plan 包含 ### How to verify### Evidence (Before & After)### Tested on 子章节
  • ## Risk & Scope — 主要风险、不在范围内的内容、破坏性变更
  • ## Linked Issues — 关联的 issue
  • 中文翻译 放在 <details> 块中

这些标题不是形式主义——它们帮助 reviewer 理解为什么要做这个改动以及如何验证,而不是靠猜。能否按模板更新 PR 正文?

Qwen Code · qwen3.7-max

.array(z.string())
.max(3, 'fallbackModel supports a maximum of 3 models')
.optional(),
proxy: z.string().min(1, 'proxy cannot be empty').optional(),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] Inconsistent whitespace validation with the Python SDK. Python uses not options.proxy.strip() which rejects whitespace-only strings like " ", but this Zod schema uses .min(1) which only rejects the empty string "". The same input passes in TypeScript but fails in Python.

Suggested change
proxy: z.string().min(1, 'proxy cannot be empty').optional(),
proxy: z.string().trim().min(1, 'proxy cannot be empty').optional(),

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

💡 Suggestion: Consolidate SDK PRs

Hi @juhuan, thanks for the comprehensive SDK work! We noticed you have 15 open PRs that all modify the same core files (transport.py, types.py, queryOptionsSchema.ts, types.ts, ProcessTransport.ts, createQuery.ts) and were created on the same day.

The problem

  • Merge conflicts: Since all 15 PRs touch the same files, whichever merges first will cause conflicts in the remaining 14.
  • Review overhead: Reviewing 15 near-identical PRs separately is inefficient and risks fatigue.
  • CI cost: 15 separate CI runs for the same lint/typecheck passes.

Suggestion: regroup into 2 PRs

We recommend closing the current 15 PRs and reopening them as 2 consolidated PRs:

PR 1 — feat(sdk): expose transport and query options in both SDKs

Covers pure SDK-side option additions (~9 current PRs):

PR 2 — feat(sdk): add control request methods to both SDKs

Covers features that also involve CLI-side ControlDispatcher changes (~4 current PRs):

This keeps a reasonable separation of concerns while eliminating the merge-conflict chain and making review much more manageable.

/cc @juhuan

@wenshao

wenshao commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Closing in favor of consolidated PRs (see suggestion comment above). Please reopen as 2 grouped PRs.

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.

3 participants