feat: add Alibaba Token Plan Qwen3.8 Max GA - #3778
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2182301d2b
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
GJC hostile-review verdict — exact head
Reviewed head: fa75518b0aedd14aa95379b77d5151f18d73a705
Authoritative dev: 732856b3ccb3fade6e9fbc17908a4fbca5a7682f
Verdict: REQUEST_CHANGES
Blocking findings:
- Qwen reasoning history is not replayed. The new model metadata sets
reasoningContentField, but Alibaba Qwen never enables the replay path gated byrequiresReasoningContentForToolCalls. Qwen documentspreserve_thinking=trueby default forqwen3.8-maxand requires every historicalreasoning_contentvalue to round-trip. The added tests capture only a first request, not a second turn or tool continuation. - Thinking can be combined with forced named/required tool choice. Qwen documents that thinking-mode models do not support forcing a specific tool. This head still advertises named support and can emit
enable_thinkingplusreasoning_effortwith forcedtool_choice. - The generator strips preview vision.
injectAlibabaTokenPlanModels()replacesqwen3.8-max-previewwithinput: ["text"], although the official Token Plan table lists visual understanding. The Responses adapter drops images when the model input metadata omitsimage. - The head is not an integrated candidate. It is 31 commits behind current
dev; GitHub reportsDIRTY, unmergeable, and unrebaseable. Its 22 successful and 6 skipped checks cover this conflicting head, not a conflict-resolved successor.
The GA identity, endpoint, 1M context, 131,072 output limit, Chat-vs-Responses split, and minimal→low / high|max→xhigh mappings are otherwise supported by official Qwen documentation. Preserving preview-based built-in profiles is also within the stated scope. No owner-only product decision is required for the fixes above.
Required before re-review: rebase and regenerate; preserve/replay Qwen reasoning history in multi-turn/tool flows; encode a valid forced-tool/thinking policy; preserve preview image capability; add payload regressions for those boundaries; obtain exact-head CI.
PR-3778 head=fa75518b0aedd14aa95379b77d5151f18d73a705 dev=732856b3ccb3fade6e9fbc17908a4fbca5a7682f evidence-sha256=49a167c7f3d5add3bb7885a6c26b1fd57bcd95b48e47f6909bc14bc3d0a68fd7 verdict=REQUEST_CHANGES
Token Plan exposes the GA model alongside preview, but dynamic catalog metadata can report stale limits and the Qwen Chat branch dropped reasoning_effort after choosing enable_thinking. Pin the official Chat contract and send both Qwen reasoning controls while preserving the preview model's Responses transport. Lore-id: 8d31f4a2 Constraint: preserve qwen3.8-max-preview Responses routing and existing built-in profiles Rejected: replace preview profile selectors | changes Token Plan credit economics without a separate product decision Confidence: high Scope-risk: moderate Reversibility: clean-revert Tested: 49 focused AI tests and @gajae-code/ai check Not-tested: sustained authenticated long-context load
The fixed Alibaba onboarding preset omitted the newly GA Qwen model and the already bundled DeepSeek Flash executor, so a fresh setup could not select the full supported catalog. Expose both models with explicit per-model transports and document why built-in profiles continue to use preview. Lore-id: c47ae610 Constraint: keep existing Alibaba profile selectors on qwen3.8-max-preview Rejected: migrate built-in profiles to GA | preview promotion and Token Plan credit rates differ Confidence: high Scope-risk: low Reversibility: clean-revert Tested: 20 onboarding tests, 70 profile tests, and @gajae-code/coding-agent check Not-tested: interactive TUI onboarding against a live account
Alibaba onboarding creates a same-id custom model whose explicit provider compat replaces bundled transport metadata. Without canonical detection, minimal and high reasoning escaped to Token Plan as unsupported literals. Teach the Alibaba Qwen3.8 Max compat detector its native aliases and cover the installed preset through the observable Chat payload. Lore-id: 6bf7e1a4 Constraint: scope aliases to alibaba-token-plan/qwen3.8-max Rejected: provider-level effort map | would alter GLM and DeepSeek requests Confidence: high Scope-risk: low Reversibility: clean-revert Tested: 65 focused tests plus @gajae-code/ai and @gajae-code/coding-agent checks Not-tested: authenticated fresh-preset request against Alibaba Token Plan
Address the blocking review findings on the Alibaba Token Plan GA model. - Qwen3.8 Max runs with `preserve_thinking` enabled, so every historical `reasoning_content` has to round-trip on later turns and the service validates the replayed value. Set `requiresReasoningContentForToolCalls` and clear `allowsSyntheticReasoningContentForToolCalls` so multi-turn and tool-continuation payloads replay real reasoning instead of dropping it or substituting a "." placeholder. - Qwen thinking mode cannot be combined with forcing a specific tool. Enable `disableReasoningOnForcedToolChoice` for the GA model and make that path actually clear the transport's thinking switch: the previous block only deleted `reasoning_effort`/`reasoning`, leaving `enable_thinking: true` (and `chat_template_kwargs.enable_thinking`) on the wire. - Token Plan documents visual understanding for `qwen3.8-max-preview`, but the generator pinned it to text-only and the Responses adapter drops image parts when `input` omits `image`. Pin the documented modality. Both the generator injection and the runtime compat detection carry the same policy, so a discovery-sourced catalog entry gets it too. Adds payload regressions for open vs. forced vs. required tool choice, for tool-call and plain-turn reasoning replay, for the no-reasoning empty-string fallback, and for the preserved preview image modality.
fa75518 to
89cb0e3
Compare
|
Rebased and all four blocking findings are addressed at head 1. Qwen reasoning history is not replayed. GA now sets 2. Thinking combined with forced tool choice. GA now sets I kept forced tool choice working with thinking suppressed rather than demoting 3. Preview vision stripped. All three policies are encoded in 4. Not an integrated candidate. Rebased onto Local evidence: 124 focused |
|
@codex review Ready for re-review at head @Yeachan-Heo I cannot add reviewers from a fork, so flagging here instead. Details are in the review-response comment above; the changed contract is:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89cb0e337b
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /(^|\/)anthropic\//i.test(model.id); | ||
| const isAlibaba = baseUrl.includes("dashscope"); | ||
| const isQwen = model.id.toLowerCase().includes("qwen"); | ||
| const isAlibabaTokenPlanQwen38Max = provider === "alibaba-token-plan" && model.id === "qwen3.8-max"; |
There was a problem hiding this comment.
For custom providers that point at the Token Plan endpoint, including the documented providers: ali example in docs/models.md, provider is not alibaba-token-plan, so this special case stays off even though the wire model is qwen3.8-max. Those models then skip the required reasoning_content replay and forced-tool thinking suppression, and the matching provider check in buildParams also skips reasoning_effort, so low/medium thinking or tool-continuation requests use the broken generic Qwen path. Key this off the Token Plan base URL/model id or the resolved compat fields instead of only the built-in provider id.
Useful? React with 👍 / 👎.
`docs/models.md` documents pointing a user-defined `providers:` entry at the Token Plan endpoint. Such a model's provider id is arbitrary, so matching only `provider === "alibaba-token-plan"` left it on the generic Qwen path: no `reasoning_content` replay, no forced-tool thinking suppression, and no mapped `reasoning_effort` — the exact breakage this branch fixes for the built-in provider. Add `detectAlibabaTokenPlanQwen38()`, which matches the Token Plan host or the built-in provider id together with the effective wire model id (`wireModelId ?? id`), and route both the compat detector and the `buildParams` reasoning-effort gate through it. A Token Plan Qwen3.8 model now also resolves `thinkingFormat: "qwen"` even when the local id no longer contains "qwen". The endpoint check is deliberately scoped to the Qwen3.8 predicate rather than widening `isAlibaba`: that flag also feeds `isNonStandard` and the thinking format, so broadening it moved GLM-5.2 and DeepSeek V4 Flash on the same endpoint onto the Qwen branch and dropped their `reasoning_effort`. Adds regressions for the custom-provider path: compat contract, wire-id rename, a non-Qwen3.8 model on the same endpoint staying untouched, effort mapping, forced-tool suppression, and tool-call reasoning replay.
|
Codex P2 addressed in The finding is correct and it was broader than the compat detector: the matching Added One note on scope, since the obvious wider fix is wrong: I first tried folding the Token Plan host into New regressions on the custom-provider path: compat contract, wire-id rename with a non-Qwen local id, a non-Qwen3.8 model on the same endpoint staying untouched, effort mapping, forced-tool suppression, and tool-call reasoning replay. 135 focused |
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Adversarial Exact-Head Review — PR #3778
Head reviewed: 27073dd83ad38a30a91b21480601336341e20ddd
Base: 732856b3ccb3fade6e9fbc17908a4fbca5a7682f (dev)
Scope: 5 commits, 14 files, +665/-67
Attack surfaces — all clean
1. Provider catalog/auth routing/model IDs ✅
provider-presets.json:modelApirouting matches actual model definitions exactly —qwen3.8-max→openai-completions,qwen3.8-max-preview→openai-responses, GLM-5.2/DeepSeek-V4-pro/flash →openai-completions. All 5 models listed correctly.- Credential safety: preset references only
apiKeyEnv: "ALIBABA_TOKEN_PLAN_API_KEY"(env var name, never a value). No hardcoded secrets anywhere in the diff. Test fixtures use"test-key"only. - Auth routing test confirms
getEnvApiKey("alibaba-token-plan")resolvesALIBABA_TOKEN_PLAN_API_KEYexclusively.
2. Endpoint-based contract detection (new commit 27073dd8) ✅
The new detectAlibabaTokenPlanQwen38() correctly replaces provider-id-only matching with endpoint+wire-model-id matching, so user-defined providers: entries pointing at maas.aliyuncs.com get the Qwen3.8 contract. Verified:
- Matches by built-in provider id OR endpoint host (
maas.aliyuncs.com) — no false-positive risk since no other provider uses this domain. - Uses
wireModelId ?? idto handle renamed models (tested). - Scoped to
isAlibabaTokenPlanQwen38Max(notisAlibaba) — deliberately avoids wideningisNonStandard, which would have incorrectly moved GLM-5.2 and DeepSeek V4 Flash off theirreasoning_effortpath. Commit message documents this reasoning explicitly. - Unrelated models on the same endpoint (
qwen3.7-max) correctly left untouched (tested). resolvedBaseUrlis consistently passed throughcreateClient→buildParams→detectAlibabaTokenPlanQwen38andgetCompat→resolveOpenAICompat→detectOpenAICompat— all see the same resolved URL.
3. Forced-tool-choice / thinking wire params ✅
enable_thinking: falsesuppression under forced tool choice is the load-bearing fix: without it, Qwen3.8 Max GA would 400 on every forced-tool turn. Theqwenandqwen-chat-templateformat branches in the disable block are both correct.reasoning_effortmapped throughreasoningEffortMap(minimal→low, high→xhigh, max→xhigh; low/medium/xhigh passthrough) — consistent across all four sources (models.json,generate-models.ts,model-thinking.ts,detectOpenAICompat).
4. Thinking-history replay (preserve_thinking) ✅
requiresReasoningContentForToolCalls: true+allowsSyntheticReasoningContentForToolCalls: falsetriggers exact-replay tiers 1 (recover from thinking blocks) and 2 (empty-string fallback), never tier 3 (synthetic".").needsReasoningOnAllTurnsensures presence on every assistant turn, not just tool-call turns. Tested for both paths.
5. Profile boundaries ✅
- Built-in Alibaba profiles (
balanced/pro/qwenmaxxing) correctly remain onqwen3.8-max-preview; GA and preview coexist. Docs explicitly state "changing profile defaults is a separate product/cost decision."
6. Generated catalogs and CI ✅
models.jsonqwen3.8-maxentry cross-verified across 4 sources — all consistent.docs-index.generated.tsregenerated → zero diff.check:schemas,check-visible-definitions— pass.tsc+biomeclean onpackages/ai.- All 19 PR-specific tests pass (13 original + 6 custom-provider regressions). 110/110 across all 4 affected test files.
Issues found
None blocking. 8 pre-existing test failures in the broader AI suite are sandbox environment contamination (ANTHROPIC_BASE_URL/OPENAI_BASE_URL set to api.layofflabs.com) — unrelated to this PR.
╔══════════════════════════════════════════════════════════════╗
║ PR #3778 — Qwen3.8 Max GA: Adversarial Review Verdict ║
╠══════════════════════════════════════════════════════════════╣
║ Reviewer: GLM-DeepSeek (adversarial exact-head) ║
║ Base: 732856b3 (dev) ║
║ Head: 27073dd8 ║
║ Commits: 5 Files: 14 +665/-67 ║
║ ║
║ VERDICT: ✅ APPROVE ║
║ ║
╠══════════════════════════════════════════════════════════════╣
║ Provider catalog/auth/model IDs ✅ CLEAN ║
║ Endpoint-based contract detection ✅ SOUND ║
║ Forced-tool / thinking wire params ✅ ESSENTIAL FIX ║
║ reasoning_content replay (preserve) ✅ CORRECT ║
║ Profile boundaries ✅ UNCHANGED ║
║ Generated catalogs + CI gates ✅ IN SYNC ║
║ Credential safety ✅ NO LEAKS ║
║ Test coverage (110/110 PR-specific) ✅ ADEQUATE ║
╠══════════════════════════════════════════════════════════════╣
║ Blocking issues: 0 ║
║ Pre-existing failures: 8 (sandbox env, unrelated) ║
║ ║
║ Reviewed without mutation, merge, CI control, or release. ║
║ ║
║ Signed: GLM-DeepSeek adversarial reviewer ║
║ Date: 2026-08-05 ║
╚══════════════════════════════════════════════════════════════╝
|
Triage: closing as superseded by the already-merged #3827 ( Important reconciliation note for the maintainer (not a blocker for this PR) — the two PRs disagree on the GA model identity and transport, and they cannot both be correct:
Neither id was validated against a live Token Plan catalog (the #3827 models.json changes were fallback-injected only, and this PR's reasoning tests are serializer-level unit tests). The established preview id Closing per the triage directive (duplicative of merged work); the transport/capability detail above is preserved here for the maintainer. |
2 participants
What
alibaba-token-plan/qwen3.8-maxas the GA model overopenai-completionswith the official 1,000,000-token context window, 131,072-token output limit, reasoning, and text/image input.qwen3.8-max-previewon its existingopenai-responsestransport when public catalog data is stale or changes routing. The preview keeps its documented visual-understanding input.enable_thinkingand the mappedreasoning_effortfor Alibaba Qwen3.8 Chat requests. Native efforts arelow,medium, andxhigh; GJC aliasesminimal -> lowandhigh|max -> xhigh.preserve_thinkingenabled, sorequiresReasoningContentForToolCallsis set andallowsSyntheticReasoningContentForToolCallsis cleared. Multi-turn and tool-continuation payloads carry the real priorreasoning_contentinstead of dropping it or substituting a.placeholder the service rejects.disableReasoningOnForcedToolChoiceis enabled for GA and that path now actually clears the transport's thinking switch. Previously it deleted onlyreasoning_effort/reasoning, leavingenable_thinking: true(andchat_template_kwargs.enable_thinking) on the wire.deepseek-v4-flash-0731model to the fixed Alibaba Token Plan onboarding preset.Both the generator injection (
injectAlibabaTokenPlanModels,applyGeneratedModelPolicy) and runtime compat detection (detectOpenAICompat) carry the same policy, so a discovery-sourced catalog entry gets it too.Why
QwenCloud now documents
qwen3.8-maxas a GA Token Plan model alongside the separately supported preview model. The current generated catalog can retain incorrect GA metadata, while the Chat Completions Qwen branch emitsenable_thinkingand skips the genericreasoning_effortbranch in the sameelse ifchain.Official contract references:
Review findings addressed
requiresReasoningContentForToolCalls: true+allowsSyntheticReasoningContentForToolCalls: falseon GA, in both the generator policy anddetectOpenAICompat. Regressions cover a tool-call continuation turn, a plain assistant turn, and the no-reasoning empty-string fallback.disableReasoningOnForcedToolChoice: trueon GA, and the suppression block inbuildParamsnow clearsenable_thinking/chat_template_kwargs.enable_thinkingfor both Qwen thinking formats. Regressions coverauto, a named tool, andrequired.qwen3.8-max-previewkeepsinput: ["text", "image"]in both the injection metadata andapplyGeneratedModelPolicy, so stale discovery cannot re-strip it.dev(732856b3c); the generated docs index conflict was resolved by regeneration, not by hand. GitHub now reportsMERGEABLE.models.jsoncarries only the three Alibaba Token Plan deltas — a fullgenerate-modelsrun also pulls unrelated live-catalog churn, which is deliberately not included here.providers:entry aimed at the Token Plan endpoint carries an arbitrary provider id, so it fell through to the generic Qwen path.detectAlibabaTokenPlanQwen38()now matches the Token Plan host or the built-in provider id together with the effective wire id (wireModelId ?? id), and both the compat detector and thebuildParamseffort gate use it. Scoped to that predicate on purpose - wideningisAlibabaalso feedsisNonStandard/thinkingFormatand broke GLM-5.2 and DeepSeek V4 Flash on the same endpoint.providers:entry aimed at the Token Plan endpoint carries an arbitrary provider id, so it fell through to the generic Qwen path.detectAlibabaTokenPlanQwen38()now matches the Token Plan host or the built-in provider id together with the effective wire id (wireModelId ?? id), and both the compat detector and thebuildParamseffort gate use it. Scoped to that predicate on purpose — wideningisAlibabaalso feedsisNonStandard/thinkingFormatand broke GLM-5.2 and DeepSeek V4 Flash on the same endpoint.Testing
bun testacross the 10 affectedpackages/aisuites — 135 passed, 0 failedbun --cwd=packages/ai run check— biome + tsc cleanbun --cwd=packages/coding-agent run check— biome + tsc cleanbun test packages/ai— identical pass/fail counts before and after this change set (the residual failures are this Windows host's pre-existingEBUSYtemp-dir teardown and native-toolchain gaps, not regressions)bun test packages/coding-agent/test/provider-onboarding.test.ts— the Alibaba Token Plan registry case passes all 34 assertions; only the Windowsfs.rmEBUSYteardown fails on this host. Canonical status is left to PR CI.GJC verdict
No independent exact-head architect/critic/human review has run on this head, so this is intentionally
needs-humanrather than self-approved. The hash below issha256(git diff origin/dev...HEAD)at head27073dd83.devbun checkpasses (per-packagecheckpasses; the repo-wide run needs a Rust toolchain and an SDK broker this Windows host lacks)