fix(cli): warn on ignored provider generation config#3883
Conversation
Warn when a selected provider model has top-level model.generationConfig fields that will not apply because the provider entry is sealed. Document the required local-model placement for contextWindowSize and related generation config fields. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
wenshao
left a comment
There was a problem hiding this comment.
Overall the change is straightforward and the user-facing intent is valuable: surfacing silently-ignored config at startup is exactly the kind of feedback users want when their settings don't take effect. Model resolution behavior is unchanged, so the risk surface is small. The README/docs additions are a nice complement — they make the "sealed package" rule for modelProviders much more discoverable.
A few non-blocking suggestions inline:
GENERATION_CONFIG_WARNING_FIELDSis a manual coupling withContentGeneratorConfigand may not be complete (proxy/userAgent).- The plural/singular handling in the warning template can be simplified.
- The new tests don't pin the singular wording branch (
is/this field/it), so a regression that collapses to always-plural would pass.
Nothing blocking from my side.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still has pending checks (Post Coverage Comment). — gpt-5.5 via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅ — gpt-5.5 via Qwen Code /review
* fix(cli): warn on ignored provider generation config Warn when a selected provider model has top-level model.generationConfig fields that will not apply because the provider entry is sealed. Document the required local-model placement for contextWindowSize and related generation config fields. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address provider config warning review Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): narrow provider config warning fields Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(cli): warn on ignored provider generation config Warn when a selected provider model has top-level model.generationConfig fields that will not apply because the provider entry is sealed. Document the required local-model placement for contextWindowSize and related generation config fields. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address provider config warning review Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): narrow provider config warning fields Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Summary
Validation
model.generationConfig.contextWindowSizeand no matching provider-level value.git diff --checkreported no whitespace issues, and the tmux TUI run displayed the startup warning.npm run buildemitted only the existing Browserslist database age warning.Scope / Risk
Testing Matrix
Testing matrix notes:
Linked Issues / Bugs
Related to #3878