Conversation
github-actions
bot
added
the
type: feature
![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: 2ba45bace1
ℹ️ 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".
| model: &ModelId, | ||
| context: ChatContext, | ||
| ) -> ResultStream<ChatCompletionMessage, anyhow::Error> { | ||
| let request = Request::from(context).model(model.clone()); |
There was a problem hiding this comment.
This sends the selected model ID straight to Qwen without any provider-specific transformation, even though the same commit introduces QwenTransformer to map internal aliases like coder-model/vision-model to upstream model names. Because QwenProvider does not run through ProviderPipeline (or otherwise apply that transformer), configured aliases are sent unmapped and can produce upstream unknown-model errors during chat requests.
Useful? React with 👍 / 👎.
| let token_response = QwenHttpProvider::poll_for_token( | ||
| &self.config, | ||
| &ctx.request.device_code, | ||
| code_verifier.as_str(), | ||
| ) |
There was a problem hiding this comment.
Qwen device-flow completion performs only one token request here, so any normal delay while the user opens the verification URL and approves access will surface as an immediate OAuth failure (for example authorization_pending). Other device flows in this codebase use a polling loop with timeout/interval handling, but this path lacks that behavior, making interactive OAuth completion brittle or consistently failing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR adds support for the Qwen LLM provider, implementing OAuth device flow authentication with PKCE and OpenAI-compatible API integration.
Changes:
- Added Qwen provider implementation with OAuth device flow and PKCE support
- Implemented custom HTTP provider for Qwen's authentication requirements
- Added model transformer for mapping internal model IDs to Qwen API model names
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/paws_services/src/provider/qwen.rs | New Qwen provider implementation with chat and models methods |
| crates/paws_services/src/provider/mod.rs | Added qwen module declaration |
| crates/paws_services/src/provider/client.rs | Integrated QwenProvider into client builder and enum |
| crates/paws_repo/src/provider.json | Added Qwen provider configuration with OAuth device flow settings |
| crates/paws_infra/src/auth/util.rs | Added PKCE code verifier/challenge generation and token parsing with resource_url |
| crates/paws_infra/src/auth/strategy.rs | Added OAuthDeviceQwenStrategy for device flow with PKCE |
| crates/paws_infra/src/auth/http/qwen.rs | New Qwen HTTP provider implementing custom device code flow with PKCE |
| crates/paws_infra/src/auth/http/mod.rs | Exposed qwen module and changed visibility to pub |
| crates/paws_infra/Cargo.toml | Added dependencies for PKCE (rand, sha2, base64) |
| crates/paws_domain/src/provider.rs | Added QWEN provider ID and Qwen response type |
| crates/paws_domain/src/auth/auth_context.rs | Added pkce_verifier field to DeviceCodeRequest |
| crates/paws_app/src/dto/openai/transformers/qwen.rs | New transformer for Qwen model ID mapping |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| code_challenge_method: "S256".to_string(), | ||
| }; | ||
|
|
||
| let client = Self::build_http_client_internal(config)?; |
There was a problem hiding this comment.
The method build_http_client_internal is a thin wrapper around build_http_client. Consider calling build_http_client(config.custom_headers.as_ref())? directly to reduce indirection.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
also
crates/paws_infra/src/auth/http/qwen.rs
code_verifier: code_verifier.to_string(),
};
let client = Self::build_http_client_internal(config)?;
https://github.com/apps/copilot-pull-request-reviewer AI
yesterday
The method build_http_client_internal is a thin wrapper around build_http_client. Consider calling build_http_client(config.custom_headers.as_ref())? directly to reduce indirection.
| code_verifier: code_verifier.to_string(), | ||
| }; | ||
|
|
||
| let client = Self::build_http_client_internal(config)?; |
There was a problem hiding this comment.
The method build_http_client_internal is a thin wrapper around build_http_client. Consider calling build_http_client(config.custom_headers.as_ref())? directly to reduce indirection.
manthanabc
force-pushed
the
main
branch
2 times, most recently
from
a7daadd to
60d6447
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@manthanabc I've opened a new pull request, #94, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Initial plan * Remove build_http_client_internal wrapper to reduce indirection Co-authored-by: manthanabc <48511543+manthanabc@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: manthanabc <48511543+manthanabc@users.noreply.github.com>
|
Todo: fix broken function calling |
No description provided.