Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough本次改动调整了 Changes温度解析优先级修复
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pytests/utils_test/test_effective_temperature.py (1)
1-54: 🎯 Functional Correctness | 🔵 Trivial测试覆盖了新的四级优先级分支,整体结构清晰。
不过目前用例未覆盖
model_info.temperature为空、但extra_params["temperature"]与调用方显式temperature同时存在时的冲突场景——该场景与src/llm_models/utils_model.py中_resolve_effective_temperature的优先级设计缺口直接相关,已在该文件对应位置提出。建议在核心逻辑修正后同步补充此分支的测试。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pytests/utils_test/test_effective_temperature.py` around lines 1 - 54, 补充 `_resolve_effective_temperature` 的冲突场景测试:当前 `test_effective_temperature.py` 只覆盖了单一路径优先级,未验证 `model_info.temperature` 为空时,`extra_params["temperature"]` 与调用方显式 `temperature` 同时存在的分支。请在 `test_model_level_temperature_overrides_explicit_temperature`、`test_explicit_temperature_used_when_model_level_absent` 等现有用例旁新增一个测试,直接针对 `LLMOrchestrator._resolve_effective_temperature`,明确断言该场景下应遵循你在 `src/llm_models/utils_model.py` 中修正后的优先级规则。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pytests/utils_test/test_effective_temperature.py`:
- Around line 1-54: 补充 `_resolve_effective_temperature` 的冲突场景测试:当前
`test_effective_temperature.py` 只覆盖了单一路径优先级,未验证 `model_info.temperature`
为空时,`extra_params["temperature"]` 与调用方显式 `temperature` 同时存在的分支。请在
`test_model_level_temperature_overrides_explicit_temperature`、`test_explicit_temperature_used_when_model_level_absent`
等现有用例旁新增一个测试,直接针对 `LLMOrchestrator._resolve_effective_temperature`,明确断言该场景下应遵循你在
`src/llm_models/utils_model.py` 中修正后的优先级规则。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1c1b73f4-a005-40fc-a1ec-0351d5c5b9e8
📒 Files selected for processing (3)
pytests/utils_test/test_effective_temperature.pysrc/config/model_configs.pysrc/llm_models/utils_model.py
|
已补充该场景的用例:模型级温度为空时,调用方显式温度优先于 extra_params 中的默认值(test_explicit_temperature_overrides_extra_params_default)。 |
请填写以下内容
main分支 禁止修改,请确认本次提交的分支 不是main分支src/A_memorix,我确认已阅读src/A_memorix/MODIFICATION_POLICY.md,不涉及则无需勾选模型列表里给模型配置的自定义温度目前会被任务侧显式传入的温度覆盖。像 Kimi-K2.6 这类只接受固定温度的模型,即使已经在模型配置里设置了 temperature=0.6,A_Memorix 证据分类、各类 learner、WebUI 的模型测试(硬编码 temperature=0.0)等调用点仍然按自己的温度发请求,直接返回 400(invalid temperature),并触发无意义的模型切换。
原因在
LLMOrchestrator._resolve_effective_temperature的优先级:调用方显式传参 > 模型级配置 > extra_params > 任务配置。#1820 把 A_memorix 的分类温度做成了可配置,但配置出来的值同样属于“调用方显式传入”,还是会压过模型级设置,所以 #1833 在其合并之后仍然出现。本次把模型级 temperature 提到最高优先级,在解析函数单点修复,覆盖所有调用点;
ModelInfo.temperature的字段说明同步改为“会覆盖任务配置和调用方显式传入的温度”。测试:新增
pytests/utils_test/test_effective_temperature.py覆盖四级优先级取值;全量 pytest 与 dev 基线对比无新增失败。其他信息
Summary by CodeRabbit
新功能
temperature时,将优先使用该值,而不再以调用方传入的temperature为主。测试
extra_params.temperature同时存在时的覆盖关系)。