Fix compile error by #2615#2619
Conversation
|
Thanks @shenjackyuanjie for taking the time to contribute. This repository is currently observing a maintainer-managed contribution gate in dry-run mode, so this pull request is staying open. When enforcement is enabled, pull requests from contributors who are not listed in Please read |
There was a problem hiding this comment.
Code Review
This pull request adds support for the SiliconflowCN provider across the CLI, config, and TUI modules, mapping it to the appropriate environment variables and slots. It also cleans up duplicate match arms for SiliconflowCn in the configuration and applies general formatting improvements. A review comment suggests grouping consecutive match arms that return None in crates/tui/src/tui/notifications.rs to simplify the code and improve readability.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| | ContentBlock::CodeExecutionToolResult { .. } => None, | ||
| | ContentBlock::ImageUrl { .. } => None, | ||
| ContentBlock::ImageUrl { .. } => None, |
There was a problem hiding this comment.
Since both match arms return None, they can be grouped together into a single match arm to simplify the code and improve readability.
| | ContentBlock::CodeExecutionToolResult { .. } => None, | |
| | ContentBlock::ImageUrl { .. } => None, | |
| ContentBlock::ImageUrl { .. } => None, | |
| | ContentBlock::CodeExecutionToolResult { .. } | |
| | ContentBlock::ImageUrl { .. } => None, |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR appears to primarily standardize/format match arms around the Siliconflow/SiliconflowCn variants and add CLI handling for the SiliconflowCN provider kind.
Changes:
- Reformats several
match/matches!usages involvingSiliconflow/SiliconflowCnacross TUI/config/client code. - Simplifies a
ContentBlock::ImageUrlmatch arm formatting in the session picker and notifications. - Adds CLI mappings for
ProviderKind::SiliconflowCN(slot + env var list).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/tui/src/tui/ui.rs | Reformats provider display-name match arm for Siliconflow. |
| crates/tui/src/tui/session_picker.rs | Condenses ImageUrl match arm to a single line. |
| crates/tui/src/tui/notifications.rs | Adjusts ImageUrl match arm formatting in assistant text extraction. |
| crates/tui/src/main.rs | Reformats provider match arms related to Siliconflow/SiliconflowCn. |
| crates/tui/src/config.rs | Reformats parsing/normalization and removes duplicate SiliconflowCn match arms. |
| crates/tui/src/client.rs | Reformats provider match arms in apply_reasoning_effort. |
| crates/config/src/lib.rs | Reformats ProviderKind::Siliconflow/SiliconflowCN match arms. |
| crates/cli/src/lib.rs | Adds ProviderKind::SiliconflowCN slot/env-var handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ProviderKind::Sglang => "sglang", | ||
| ProviderKind::Vllm => "vllm", | ||
| ProviderKind::Ollama => "ollama", | ||
| ProviderKind::SiliconflowCN => "siliconflow", | ||
| } |
|
Thanks @shenjackyuanjie for catching #2615 and sending the narrow compile repair. I reproduced the failure on main with I am carrying this repair into the v0.8.52 stabilization branch and will keep #2618 open until the fix is merged/released. Sorry this landed broken on main. |
|
Closing as superseded by #2626, which is now merged on |
Summary
Testing
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-featurescargo test --workspace --all-featuresfixing #2618
Greptile Summary
This PR fixes compile errors introduced by #2615, which added the
SiliconflowCN/SiliconflowCnprovider variant. The fixes are mechanical: removing duplicatematcharms, adding missing exhaustive arms, correcting a spurious leading|on a pattern, and reformatting long lines to satisfyrustfmt.crates/cli/src/lib.rs: Adds the two missingProviderKind::SiliconflowCNarms inprovider_slotandprovider_env_vars, resolving non-exhaustive match errors.crates/tui/src/config.rs: Removes multiple duplicateSiliconflowCnarms (some at wrong indentation levels) that caused "unreachable pattern" compile errors.crates/tui/src/tui/notifications.rs: Removes the erroneous leading|from theContentBlock::ImageUrlarm, and the remaining files receive formatting-only adjustments.Confidence Score: 5/5
Safe to merge — every change is either removing a duplicate unreachable match arm, adding a missing exhaustive arm, or reformatting to meet line-length limits.
All changes are narrowly scoped to fixing the broken state left by #2615: no new logic is introduced, duplicated arms are simply deduplicated, and the new SiliconflowCN arms in the CLI mirror the pattern used by every other provider.
No files require special attention. crates/tui/src/config.rs had the most edits (three sets of duplicate arm removals) but the result is straightforwardly correct.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[SiliconflowCN provider added in #2615] --> B{Compile errors in 8 files} B --> C[cli/src/lib.rs\nMissing match arms] B --> D[tui/src/config.rs\nDuplicate SiliconflowCn arms] B --> E[tui/src/tui/notifications.rs\nSpurious leading pipe] B --> F[Other files\nLine-length violations] C --> G[All checks pass:\ncargo fmt / clippy / test] D --> G E --> G F --> GReviews (1): Last reviewed commit: "Fix compile error by #2615" | Re-trigger Greptile