fix(command-code): add v4.1-flash and Qwen3.8-Flash effort ladders - #4258
fix(command-code): add v4.1-flash and Qwen3.8-Flash effort ladders#4258gitgarmin wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe provider adds reasoning-effort ladders for two Command Code models. Tests verify both authentication presets, case-insensitive lookup, and forwarding of the ChangesCommand Code model reasoning efforts
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The new models expose their intended reasoning controls and forward supported effort values correctly. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Review readiness checklist
0/4 boxes ticked. This PR stays in draft until every box above is ticked. Hygiene✅ Deterministic PR hygiene checks passed. |
Both routes are live-discovered but had no row in the official effort table, so supportedCommandCodeEffort resolved to undefined and the adapter omitted reasoning_effort entirely: a client's `max` reached /alpha/generate as no reasoning parameter at all, and neither preset advertised an effort control for them. The ladders are inferred from the same-family rows (deepseek v4: high..max; the Qwen 3.8 family: low..max). The profile pages render client-side with an empty reasoning payload, so the self-refresh path stays dead for these rows exactly as the lidge-jun#2647 block above documents. Measured live 2026-09-11 on 2.50.0: /alpha/generate accepts reasoning_effort "max" on both routes (HTTP 200), while the unpatched adapter stripped the field before the wire. Tests: bun test tests/providers/command-code-provider.test.ts (47 pass), bun test tests/providers/commandcode-provider.test.ts (8 pass), bun x tsc --noEmit (clean), bun run privacy:scan (clean).
리뷰 · 우선순위 61 / 80이 PR은 Command Code 어댑터가 살아 있는 두 경로( 왜 중요한지 짧게 말하면, 클라이언트가 사다리는 프로필 페이지에서 긁은 게 아닙니다. commandcode.ai는 클라이언트 렌더라 reasoning 페이로드가 비어 있고, 지금 라인 - 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
2417cee to
857af82
Compare
|
Thanks for the review — quick updates, since it was written against head Rebase + readiness: rebased onto Qwen3.8-Flash ladder — live confirmation (the open question): all four rungs were measured against the running 2.50.0 proxy on 2026-09-11 (
So the full On the On pinning |
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Checked 857af82. The missing table row does explain why supportedCommandCodeEffort returns undefined and strips the requested effort; adding these two rows is a focused change, not a provider endpoint/auth expansion. Your new Qwen four-rung report improves on the earlier max-only observation.
Please update the in-code provenance to distinguish that new measured report from the remaining family inference; it still says only max was measured. Keep HTTP acceptance separate from proof that the upstream actually applies a distinct reasoning level. For deepseek-v4.1-flash, the high rung is still described as inferred, not independently measured.
Add the new exact-id builder assertions for xhigh -> max and ultra omission. The older v4-flash test is not an equivalent negative control: that id is in needsAlias and accepts ultra -> max, whereas this new id deliberately is not. Preserve that difference instead of expanding ultra support by analogy. Current hosted product/typecheck evidence remains separate from READY/intake checks. No live paid request or local provider setting was changed.
lidge-jun
left a comment
lidge-jun
left a comment
There was a problem hiding this comment.
Reviewed against the current tree. The deepseek/deepseek-v4.1-flash row is well-founded and the mechanism is right: the table is the only effort source for Command Code (/provider/v1/models carries no effort contract), both presets share the same object (src/providers/registry.ts:1419 and :2307), and the lookup folds case (src/providers/command-code-efforts.ts:149-159). Dropping ultra is correct — it is absent from needsAlias (src/adapters/command-code.ts:497-500) so it would be stripped, and xhigh already clamps at :501-503.
Two things to fix, neither blocking the deepseek row.
1. The Qwen provenance comment cites a row that does not exist. The comment says the ladder is inferred from "the same-family rows above … the Qwen 3.8 family: low..max", but COMMAND_CODE_MODEL_EFFORTS has no Qwen row at all — deepseek/deepseek-v4.1-flash is the only new family member, and the fixture (tests/fixtures/commandcode-models.json) carries neither id. What the PR actually measured is max → HTTP 200. That supports max, and by the table's own convention high, but low and medium rest on nothing. Please either measure those two rungs or narrow the row to what was observed.
2. The "re-sends once without the field" claim in the description does not hold for these rows. src/adapters/command-code.ts:577-580 only retries when refreshed is defined, and the file's own comment at :24-37 documents that the profile parser is dead for client-rendered pages — which is exactly why these rows had to be hand-written. So a wrong rung is not a graceful degrade here; it is an upstream 400 on the request the user selected. That raises the cost of finding 1.
Everything else checks out: base is dev, no security surface, tests accompany the change, and the scope stays at two files. The oauth/api-key loop in the new test exercises one shared object rather than two, but that matches the existing GLM test above it, so it is not a new defect.
Merging on the maintainer path so the live routes stop losing their effort field. Finding 1 is tracked as follow-up — if the low/medium rungs turn out to be rejected upstream, narrowing the row is a one-line change.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 60f5b370cc
ℹ️ 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".
| "deepseek/deepseek-v4.1-flash": { | ||
| efforts: ["high", "max"], | ||
| profileUrl: "https://commandcode.ai/models/deepseek-v4-1-flash", |
There was a problem hiding this comment.
These new provider effort contracts change runtime behavior under src/providers/, but the commit leaves every owning structure document unchanged; structure/INDEX.md maps this area to runtime.md, subagents.md, transports/inventory.md, and providers/xai-grok.md. Update the applicable source-of-truth documentation in this change, or narrow the ownership mapping if some listed documents do not actually own this contract.
AGENTS.md reference: AGENTS.md:L33-L41
Useful? React with 👍 / 👎.
…4282) Splits the DeepSeek thinking set by who serves the route (first-party deepseek-flash vs gateway deepseek-v4.1-flash), removes the retired deepseek-v4-pro from every static roster, and excludes it from the routed catalog on live-discovery providers where deleting a row would strip capabilities instead of the model. Carries the Command Code effort ladders from #4258. Maintainer integration into dev per MAINTAINERS.md with all checks passing at the exact head.
|
Landed — thank you, @gitgarmin. Both rows are on Why it went through a carry rather than a merge of this PR: updating this branch onto current The review findings still stand as follow-ups: the Qwen provenance comment cites a same-family row that does not exist in |
lidge-jun
added
the
landed-via-maintainer
3 participants
Summary
deepseek/deepseek-v4.1-flashandQwen/Qwen3.8-Flashare live-discovered command-code routes with no row inCOMMAND_CODE_MODEL_EFFORTS. With no row,supportedCommandCodeEffortresolves toundefined, so the adapter omitsreasoning_effortentirely: a client'smaxreaches/alpha/generateas no reasoning parameter at all, and neither preset advertises an effort control for the route.Evidence
Measured live on 2026-09-11 against this machine's running 2.50.0 proxy carrying the same rows, read from the proxy request log (
requestedEffort/effectiveEffort/reasoningWireValue) and the upstream status:reasoning_effortbeforedeepseek/deepseek-v4.1-flashmaxmaxQwen/Qwen3.8-FlashmaxmaxAdapter-level probes on the same rows:
max -> max,high -> high,xhigh -> max(the generic clamp),ultra -> stripped. No alias is claimed forultra, because no profile evidence backs one the way it does for v4-pro/v4-flash.Ladder provenance
Both ladders are inferred from the same-family rows already in the table (deepseek v4:
high..max; the Qwen 3.8 family convention:low..max), not scraped from the profile pages: commandcode.ai renders those client-side with an empty reasoning payload, sorefreshCommandCodeReasoningEffortscannot self-correct these rows, exactly as the #2647 block above documents. The live 200s above are the evidence that upstream accepts these values; if a rung is ever rejected, the adapter already re-sends once without the field.Verification
bun test tests/providers/command-code-provider.test.ts- 47 pass / 0 fail (includes the new test)bun test tests/providers/commandcode-provider.test.ts- 8 pass / 0 failbun x tsc --noEmit- cleanbun run privacy:scan- cleanbun run test(full suite, macOS) - passes excepttests/codex-integration/codex-shim.test.ts, one launcher-timing test that fails identically on the unmodified dev tip (18e553a5, same as this PR base) in this environment, so it is unrelated to this diff. (The runner parallel lane reportsaborted: worker panickedfor many files under full parallelism on this machine; its isolated re-run clears them.)Checklist
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.
Summary by CodeRabbit
New Features
maxsetting.Tests