Skip to content

fix(providers): correct Qwen3.8 reasoning levels - #1304

Closed
XIQIXIQIXIQI wants to merge 1 commit into
lidge-jun:devfrom
XIQIXIQIXIQI:codex/fix-qwen38-reasoning-levels
Closed

fix(providers): correct Qwen3.8 reasoning levels#1304
XIQIXIQIXIQI wants to merge 1 commit into
lidge-jun:devfrom
XIQIXIQIXIQI:codex/fix-qwen38-reasoning-levels

Conversation

@XIQIXIQIXIQI

@XIQIXIQIXIQI XIQIXIQIXIQI commented Aug 8, 2026

Copy link
Copy Markdown

Summary

  • Correct Alibaba Token Plan's Qwen3.8-Max reasoning ladder to the officially documented low, medium, and xhigh levels, with xhigh as the default.
  • Route Qwen3.8-Max through the direct reasoning_effort field instead of the legacy Qwen thinking_budget contract in both Beijing and international presets.
  • Repair stale persisted preset metadata at registry enrichment and routing time, including case-insensitive model keys and provider-wide aliases that could otherwise leak unsupported high or max values.
  • Update the English and Simplified Chinese architecture references with the direct-effort exception.

The previous registry data classified Qwen3.8-Max like older Qwen3.x budget models. International metadata also exposed high while omitting the supported medium tier. Existing provider rows persist capability metadata, so correcting only the registry seed would leave upgraded installations on the stale wire contract.

This change makes new and existing Alibaba Token Plan configurations follow the Qwen3.8-Max documentation. Codex compatibility levels above the upstream ladder clamp to xhigh rather than reaching the provider as unsupported values.

Verification

  • bun test tests/reasoning-effort.test.ts tests/alibaba-intl-token-plan.test.ts tests/provider-registry-parity.test.ts — 89 passed, 0 failed.
  • bun run test — 10,132 passed, 7 skipped, 0 failed across 631 files.
  • bun run typecheck
  • bun run privacy:scan
  • cd docs-site && bun run build
  • git diff --check

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 54b35cfc-73cd-4c94-a3d3-1741865f922b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@github-actions github-actions Bot added the bug Something isn't working label Aug 8, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

Approved the pending Cross-platform CI run at this exact head, and it came back green.

Worth explaining the silence beforehand: contributor PRs queue their workflow runs as action_required until a maintainer releases them, and gh pr checks does not show that state — so from your side it looks like nothing is happening rather than like something is waiting on us. That was ours to clear, not yours.

To be precise about what this does and does not do: it only unblocks CI. It does not make this mergeable. The four-box readiness checklist in the description is your attestation, and the gate keeps the PR in draft until you complete it. With CI now green at your head, box 1 is provable.

If anything in the run looks wrong to you, say so and I will dig into it rather than leaving you to guess.

@XIQIXIQIXIQI
XIQIXIQIXIQI force-pushed the codex/fix-qwen38-reasoning-levels branch from 83c9805 to b921e34 Compare August 9, 2026 03:21
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ All CI tests are green on my local testing.
  • ⬜ I pushed my PR to the latest dev commit.
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

@lidge-jun

Copy link
Copy Markdown
Owner

Landed on dev as a1e4185 with your authorship preserved, plus one corrective commit on top (72ff55d).

Your finding was correct: THINKING_BUDGET_EFFORTS wrongly covered Qwen3.8-Max when the official contract uses direct reasoning_effort with low/medium/xhigh. What the patch also did, though, was delete explicit user configuration — your own test asserted it:

modelReasoningEfforts: { "QWEN3.8-MAX": ["low", "high", "xhigh"] },
expect(provider.modelReasoningEfforts["QWEN3.8-MAX"]).toBeUndefined();

This repo keeps registry defaults beneath user entries, so a deliberately configured Qwen3.8 override has to survive. The correction delivers the right contract without removing user overrides, and the tests now assert the correct behavior: the stale generated default is fixed while an explicit per-model override is preserved. Red ablation confirmed both regressions fail without the fix (the uppercase budget entry gets deleted and the custom ladder gets replaced).

Full suite green on the landed head. Thanks — the contract research and the doc links were solid; only the ownership rule needed adjusting.

@lidge-jun lidge-jun closed this Aug 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants