-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat(core): extend cross-auth fast models to agents #4153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8f47d5e
00f4ad7
e0faaf3
5f6dd02
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1861,52 +1861,35 @@ export class Config { | |||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Returns the fast model if one is configured and valid for the current auth | ||||||||||
| * type, otherwise returns undefined. Direct runtime paths use this as a | ||||||||||
| * cheaper alternative to the main session model, so it intentionally stays | ||||||||||
| * current-auth-only. | ||||||||||
| * Returns the configured fast model selector when it resolves to an available | ||||||||||
| * model. Bare selectors stay bare and authType-qualified selectors keep their | ||||||||||
| * authType prefix so selector-aware runtime paths can route cross-auth calls. | ||||||||||
| */ | ||||||||||
| getFastModel(): string | undefined { | ||||||||||
| const authType = | ||||||||||
| this.contentGeneratorConfig?.authType ?? | ||||||||||
| this.modelsConfig.getCurrentAuthType(); | ||||||||||
| if (!authType) return undefined; | ||||||||||
| const selector = this.resolveFastModelSelector(); | ||||||||||
| if (!selector) return undefined; | ||||||||||
| if (selector.authType && selector.authType !== authType) return undefined; | ||||||||||
|
|
||||||||||
| const available = this.getAllConfiguredModels([authType]); | ||||||||||
| return available.some((m) => m.id === selector.modelId) | ||||||||||
| ? selector.modelId | ||||||||||
| : undefined; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Returns the fast model for side-query paths. Unlike {@link getFastModel}, | ||||||||||
| * this can return an authType-qualified selector because BaseLlmClient can | ||||||||||
| * route a single request through a provider different from the main session. | ||||||||||
| */ | ||||||||||
| getFastModelForSideQuery(): string | undefined { | ||||||||||
| const selector = this.resolveFastModelSelector(); | ||||||||||
| if (!selector) return undefined; | ||||||||||
|
|
||||||||||
| if (selector.authType) { | ||||||||||
| const available = this.getAllConfiguredModels([selector.authType]); | ||||||||||
| return available.some((m) => m.id === selector.modelId) | ||||||||||
| ? `${selector.authType}:${selector.modelId}` | ||||||||||
| : undefined; | ||||||||||
| const available = selector.authType | ||||||||||
| ? this.getAllConfiguredModels([selector.authType]) | ||||||||||
| : this.getAllConfiguredModels(); | ||||||||||
| if (!available.some((m) => m.id === selector.modelId)) { | ||||||||||
| return undefined; | ||||||||||
|
tanzhenxin marked this conversation as resolved.
|
||||||||||
| } | ||||||||||
|
|
||||||||||
| const available = this.getAllConfiguredModels(); | ||||||||||
| return available.some((m) => m.id === selector.modelId) | ||||||||||
| ? selector.modelId | ||||||||||
| : undefined; | ||||||||||
| const rawSelector = resolveModelId(this.fastModel); | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correctness bug: double-parsing
This line re-parses Concrete failure scenario:
The returned string routes to Suggested fix: Use return selector.authType
? `${selector.authType}:${selector.modelId}`
: selector.modelId;This keeps the formatting aligned with the validation that already happened above.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] Meanwhile, Result:
Suggested change
This uses the already-resolved — qwen-latest-series-invite-beta-v28 via Qwen Code /review |
||||||||||
| return rawSelector?.authType | ||||||||||
| ? `${rawSelector.authType}:${selector.modelId}` | ||||||||||
|
tanzhenxin marked this conversation as resolved.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor:
A cleaner shape: have the parser hand back a |
||||||||||
| : selector.modelId; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private resolveFastModelSelector() { | ||||||||||
| if (!this.fastModel) return undefined; | ||||||||||
| try { | ||||||||||
| return resolveModelId(this.fastModel); | ||||||||||
| return resolveModelId(this.fastModel, { | ||||||||||
| currentAuthType: this.getContentGeneratorConfig()?.authType, | ||||||||||
| getAvailableModels: (authTypes) => | ||||||||||
| this.getAllConfiguredModels(authTypes), | ||||||||||
| }); | ||||||||||
| } catch { | ||||||||||
| return undefined; | ||||||||||
| } | ||||||||||
|
|
||||||||||
Uh oh!
There was an error while loading. Please reload this page.