Dev #161
Merged
Dev #161
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for VLLM and test-model configurations across the codebase, extends the CLI with a custom --config-path option, refactors the testing suite to run the full pipeline against multiple config files, and bumps the project and config versions.
- Introduce
VllmArgsandTestModelArgsinconfig_models.pyand wire them intoWcConfigandcreate_config_by_arg_type - Update
offline_infer.vllm_infersignature and ChatModel initialization inapi_service.py - Refactor
tests/test_full_pipeV2.pyto parameterize tests over all configs, replacing the removedtest_full_pipe.py
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| weclone/utils/config_models.py | Add VllmArgs and TestModelArgs, comment out deprecated enum items |
| weclone/utils/config.py | Handle "vllm" and "test_model" in create_config_by_arg_type |
| weclone/server/api_service.py | Pass a JSON dict to ChatModel via model_dump |
| weclone/prompts/clean_data.py | Fix markdown list numbering |
| weclone/eval/test_model.py | Use TestModelArgs.test_data_path and completion_config |
| weclone/data/qa_generator.py | Replace QaPairV2 with QaPair |
| weclone/data/models.py | Rename QaPairV2 to QaPair |
| weclone/data/clean/strategies.py | Update type hints from QaPairV2 to QaPair |
| weclone/core/inference/offline_infer.py | Extend vllm_infer and inject VllmArgs |
| weclone/cli.py | Add --config-path option (removed @click.group()) |
| tests/test_full_pipeV2.py | Restructure and parameterize full-pipeline tests |
| tests/configs/*.jsonc | Bump versions, add test_model_args and vision_api sections |
| settings.template.jsonc & pyproject.toml | Version bumps and dependency updates |
| README.md | Added a note about model-size behavior |
Comments suppressed due to low confidence (7)
weclone/cli.py:61
- The
@click.group()decorator was removed, so no CLI commands will be registered. Re-add@click.group()(and apply the option) to ensure subcommands load correctly.
@click.option("--config-path", default=None, help="指定配置文件路径，会设置WECLONE_CONFIG_PATH环境变量")
weclone/prompts/clean_data.py:15
- The bullet starts with
+3.which breaks standard markdown numbering. Change to3.to restore correct formatting.
3. **风格代表性** (Style Representativeness): 评估【回答 A】是否展现了自然、独特的人类对话风格特征。它是否仅仅是功能性的信息传递，还是带有个性化的色彩？关注点包括但不限于：是否体现了特定的语气（如友好、幽默、不耐烦、正式、脏话），是否包含口头禅、俚语、网络用语（如“yyds”、“绝绝子”）、表情符号 Emoji、颜文字、标点符号的特殊使用如“!!!”、“???”、“~”等表达、特定的缩写或短语、非标准的但一致的表达方式（如方言词汇、个人口癖）？如果包含请给予5分，不包含给予5分以下分数
README.md:45
- [nitpick] This note is unprofessional and may confuse readers. Consider revising or removing to maintain a professional project tone.
> - 7B模型很容易训练成为二傻子，14B模型勉强可以交流，32B及以上的模型效果会更好。
weclone/utils/config_models.py:126
- Using
Field(VisionApiConfig())shares one default instance across all models. Switch back todefault_factory=VisionApiConfigto ensure eachMakeDatasetArgsgets its ownVisionApiConfig.
vision_api: VisionApiConfig = Field(VisionApiConfig())
weclone/utils/config_models.py:182
- Providing a mutable default model instance can cause shared-state bugs. Use
default_factory=VllmArgsinstead of a direct instance.
vllm_args: VllmArgs = Field(VllmArgs())
weclone/utils/config_models.py:183
- Similarly, use
default_factory=TestModelArgsto avoid sharing one instance ofTestModelArgsacross allWcConfiginstances.
test_model_args: TestModelArgs = Field(TestModelArgs())
weclone/core/inference/offline_infer.py:120
json_schemais not defined in this scope, causing aNameError. Either import or compute the schema or remove this field fromextra_body.
extra_body = {"guided_json": json_schema, "enable_thinking": False}
Comment on lines
+61
to
+62
| # FULL = "full" | ||
| # FREEZE = "freeze" |
There was a problem hiding this comment.
Commented-out enum members clutter the code. If these values are deprecated, remove them or add a proper deprecation notice instead of leaving commented code.
Suggested change
| # FULL = "full" | |
| # FREEZE = "freeze" | |
Copilot uses AI. Check for mistakes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
test_model_argsandvllm_args配置项，允许自定义测试集文件fix #158 fix #83 fix #77 fix #69