Simplify Qwen guidance configuration #52
Conversation
Summary of ChangesHello @Tenos-ai, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the configuration experience for Qwen models by eliminating duplicate KSampler CFG settings. The change ensures that Qwen's sampler CFG is consistently managed through the primary default guidance setting, reducing complexity and potential user error. It also includes a robust migration path for existing configurations and updates default values to a more appropriate baseline. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request simplifies the configuration for the Qwen model by removing redundant CFG controls and driving the sampler CFG from the default guidance settings. The changes include migrating legacy settings, updating default values, and removing the obsolete configuration from the UI and templates. The implementation looks solid, especially the migration logic in settings_manager.py which correctly handles legacy values while preserving user customizations. I've added a couple of suggestions to improve code conciseness and maintainability in areas where settings are parsed.
| final_guidance = settings.get('default_guidance_qwen_edit', 2.5) | ||
| try: | ||
| final_guidance = float(final_guidance) | ||
| except (ValueError, TypeError): | ||
| final_guidance = 5.5 | ||
| final_guidance = 2.5 |
There was a problem hiding this comment.
This block for getting and converting the guidance value can be made more concise. The current implementation is a bit verbose and can be simplified to reduce redundancy.
| final_guidance = settings.get('default_guidance_qwen_edit', 2.5) | |
| try: | |
| final_guidance = float(final_guidance) | |
| except (ValueError, TypeError): | |
| final_guidance = 5.5 | |
| final_guidance = 2.5 | |
| try: | |
| final_guidance = float(settings.get('default_guidance_qwen_edit', 2.5)) | |
| except (ValueError, TypeError): | |
| final_guidance = 2.5 |
| try: | ||
| current_guidance = float(settings.get('default_guidance_qwen_edit', 5.5)) | ||
| current_guidance = float(settings.get('default_guidance_qwen_edit', 2.5)) | ||
| except (ValueError, TypeError): | ||
| current_guidance = 5.5 | ||
| current_guidance = 2.5 |
There was a problem hiding this comment.
To improve maintainability and avoid magic numbers, it's a good practice to define 2.5 as a constant, especially since it's used twice here. This makes it easier to update the value in the future and improves readability.
| try: | |
| current_guidance = float(settings.get('default_guidance_qwen_edit', 5.5)) | |
| current_guidance = float(settings.get('default_guidance_qwen_edit', 2.5)) | |
| except (ValueError, TypeError): | |
| current_guidance = 5.5 | |
| current_guidance = 2.5 | |
| DEFAULT_QWEN_EDIT_GUIDANCE = 2.5 | |
| try: | |
| current_guidance = float(settings.get('default_guidance_qwen_edit', DEFAULT_QWEN_EDIT_GUIDANCE)) | |
| except (ValueError, TypeError): | |
| current_guidance = DEFAULT_QWEN_EDIT_GUIDANCE |
![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.
ℹ️ 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".
| legacy_cfg_pairs = ( | ||
| ("qwen_ksampler_cfg", "default_guidance_qwen"), | ||
| ("qwen_edit_ksampler_cfg", "default_guidance_qwen_edit"), | ||
| ) | ||
| for legacy_key, guidance_key in legacy_cfg_pairs: | ||
| if legacy_key in settings: | ||
| legacy_value = settings.pop(legacy_key) | ||
| try: | ||
| coerced = float(legacy_value) | ||
| except (TypeError, ValueError): | ||
| coerced = None | ||
| if coerced is not None: | ||
| current_guidance = settings.get(guidance_key) | ||
| if current_guidance in (None, "") or current_guidance == default_settings.get(guidance_key): | ||
| settings[guidance_key] = coerced |
There was a problem hiding this comment.
The migration only copies qwen_ksampler_cfg/qwen_edit_ksampler_cfg into the new unified guidance keys when the existing guidance value is None, empty, or equal to _get_default_settings()’s value (now 2.5). On existing installs, default_guidance_qwen and default_guidance_qwen_edit will typically still be the previous default 5.5 while users may have customised only the KSampler CFG fields. With the current check, those customised CFG values are popped and discarded, leaving the guidance set to 5.5 after upgrade. Any user who tuned the sampler CFG but not the guidance will silently lose their configuration. Consider treating the old default (5.5) as “unmodified” or always migrating the legacy key’s value so custom sampler guidance survives the upgrade.
Useful? React with 👍 / 👎.
Summary
Testing
Codex Task